| <<<Back 1 day (to 2013/06/11) | 2013/06/12 |
paulgardiner | Robin_Watts, tor8: Did we decide that the fz_interactive removable was good to go? | 10:40.11 |
Robin_Watts | paulgardiner: Yeah, tor8 passed it last night. | 10:40.27 |
| I'll push it after my run. | 10:40.32 |
paulgardiner | Great ta | 10:40.40 |
tor8 | morning Robin_Watts and paulgardiner | 10:42.43 |
paulgardiner | hi tor8 | 10:42.59 |
tor8 | so, are we going ahead with the header file (and source file) reshuffling? | 10:43.04 |
Robin_Watts | tor8: I think we should, but I have not looked at your attempts beyond what we talked about at the meeting. | 10:43.41 |
tor8 | Robin_Watts: the source reshuffling could do with some more thought | 10:44.08 |
| the header file stuff is so far a simple enough first step | 10:44.20 |
Robin_Watts | tor8: do a branch and we can hack on it? | 10:44.26 |
paulgardiner | I can't claim to have kept completely up to date with what you have planned, but the reasoning all seemed sound. | 10:44.41 |
tor8 | tor/headers | 10:44.52 |
| I should take a few of the patches there for the makefile cleanups and backport to master | 10:45.10 |
| I'll do that and then we can look when robin gets back from his run | 10:45.21 |
paulgardiner | Might be best to backport to my master branch | 10:48.09 |
tor8 | the interactive removal branch? | 10:48.34 |
paulgardiner | yep | 10:49.32 |
Robin_Watts | let me push pauls stuff before I go. | 10:50.30 |
| pushed | 10:51.53 |
paulgardiner | ta | 10:51.57 |
Robin_Watts | np. | 10:51.59 |
| I'll look at michaels stuff when I get back. | 10:52.16 |
paulgardiner | tor8: have all header files been moved into include or just those that represent the external API of MuPDF? | 11:00.38 |
tor8 | paulgardiner: all of them (except the data_*.h in pdf/) | 11:01.01 |
| the idea is to then split them into public and module-specific-implementation-detail header | 11:01.33 |
| and put the module-imp headers with the sources | 11:01.43 |
paulgardiner | Sounds good. | 11:01.54 |
tor8 | and not have the contains-everything headers with the public/private split | 11:02.26 |
| but rather have module-public and module-private headers instead | 11:02.42 |
| and for client use, a "fitz.h" which includes all the public headers | 11:02.57 |
| and then be strict about only including the public headers that each source file needs | 11:03.15 |
| but the later steps are all incremental, no need to change them all at once | 11:03.31 |
paulgardiner | Does include contain only mupdf. If so, is it planned that include might contain other subdirectories? | 11:04.00 |
tor8 | it would be good to make a list of modules though, and rearrange all the source files and split the headers before too much other work is done | 11:04.02 |
| paulgardiner: the idea is to have only one top level include directory so we don't pollute the /usr/include namespace too much | 11:04.40 |
| so we'd install our headers into /usr/include/mupdf/*.h and then users would #include "mupdf/*.h" | 11:05.00 |
paulgardiner | Right. I get it. | 11:05.15 |
tor8 | I think naming the directory mupdf rather than fitz will serve us better | 11:05.26 |
| for name recognition | 11:05.31 |
paulgardiner | Definitely | 11:05.39 |
tor8 | we should also install libmupdf.a rather than libfitz.a :) | 11:05.47 |
paulgardiner | Right. | 11:05.56 |
sebras | tor8: wouldn't it make sense to have mupdf.h be the include-all-header in that case? wrt. name recognition... | 11:42.06 |
tor8 | sebras: possibly. | 11:46.37 |
| but that'd make the fz_ prefix a bit cryptic :) | 11:46.47 |
sebras | tor8: indeed. maybe this is the incentive necessary to change from fz_ to mu_? | 11:48.00 |
| tor8: if you like to keep it consistent. | 11:48.14 |
sebras | likes the name fitz though... :-/ | 11:48.28 |
tor8 | I don't think changing the prefix to mu_ is warranted | 11:52.14 |
| too much pain for everyone, not enough gain | 11:52.25 |
sebras | tor8: maybe for 2.0 | 12:10.01 |
| tor8: when you rename fitz.h to mupdf.h. ;) | 12:10.10 |
| or even mu.h | 12:10.15 |
Robin_Watts | Personally, I'd favour include/mupdf/fitz/{fitz.h,pixmap.h, etc} | 12:27.12 |
| Or maybe: include/mupdf/fitz.h, as the top level one, but include/mupdf/fitz/{context,pixmap}.h | 12:28.31 |
| That way people can do things like: #include "mupdf/fitz/pixmap.h" or #include "mupdf/pdf/annotations.h" | 12:29.21 |
tor8 | Robin_Watts: yeah, subdirs for modules might be okay, but do you think we'll have enough submodules to make it worthhwile? | 12:41.54 |
| Robin_Watts: paulgardiner: several patches on tor/master for review | 12:43.33 |
Robin_Watts | tor8: I can imagine us running into a case where we have fitz/blah.h and pdf/blah.h | 12:43.36 |
tor8 | Robin_Watts: right. and you definitely prefer fitz/blah.h over fitz-blah.h IIRC. | 12:44.10 |
Robin_Watts | I like the fact it's a clear delineation of what level things are at. | 12:44.20 |
tor8 | my concern is that we'll end up with dozens of subdirs with only one file each in it | 12:44.25 |
Robin_Watts | tor8: And this is bad because ? | 12:44.34 |
tor8 | awkward file navigation | 12:44.41 |
| having to open a dozen subdirs before you can open the files you need | 12:44.56 |
Robin_Watts | We could have the convention that mupdf/X.h includes all the headers in mupdf/X/ | 12:45.51 |
tor8 | but it's only a minor inconvenience | 12:45.58 |
| Robin_Watts: that would be a nice convenience | 12:46.16 |
Robin_Watts | hence mupdf/{fitz,pdf,xps,cbz}.h would make sense. | 12:46.19 |
tor8 | mupdf/fitz.h includes mupdf/fitz/*.h | 12:46.36 |
| etc | 12:46.48 |
Robin_Watts | yeah. | 12:46.54 |
tor8 | so do we want to split fitz into smaller bits? | 12:48.56 |
| i.e. separate out the resource, device, stream, etc into separate modules | 12:49.26 |
paulgardiner | tor8: first two look fine | 12:49.34 |
| tor8: begin/end look fine too | 12:54.10 |
tor8 | paulgardiner: thanks. | 12:56.10 |
paulgardiner | span soup best checked by Robin_Watts, i think | 12:56.59 |
sebras | tor8: Robin mentioned pixmap.h above, which seems to be reasonable to me. | 12:57.00 |
| tor8: I guess it depends on how tangled the data structures are when they are used as arguments to functions. | 12:57.32 |
Robin_Watts | tor8: yeah, I think some degree of splitting would be worthwhile. | 12:57.49 |
sebras | i.e. if we have foo(A a, B b) then it might not make sense to separate A and B into separate headers. | 12:58.03 |
tor8 | Robin_Watts: the benefit of mupdf/X.h and mupdf/X/Y.h means the split is transparent to most users | 12:58.27 |
| sebras: same problem as deciding which class does a method belong to... | 12:59.03 |
sebras | tor8: indeed. but lets not discuss oop. I have enough of that at work... ;) | 12:59.29 |
tor8 | foo(A a, B b) might need to #include "B.h" :) | 12:59.33 |
| like fz_write_png, which would belong to pixmap.h but use output.h | 12:59.57 |
sebras | tor8: do you want to separate everything into one header per .c-file? | 13:00.02 |
tor8 | sebras: no, not that far | 13:00.10 |
| Robin_Watts: I suggest we do this in a few steps: | 13:00.59 |
| 1) merge fitz.h and fitz-internal.h | 13:01.04 |
| 2) split fitz.h into submodules (and pdf.h etc) | 13:01.15 |
| 3) move private submodule stuff into -imp headers include/mupdf/fitz/pixmap.h -> pixmap-imp.h in the source directories | 13:02.01 |
Robin_Watts | sebras: No, the point of the headers is to separate declaration from implementation. | 13:02.10 |
| The number of .c files used is a private matter. | 13:02.22 |
| tor8: yeah, it looks like a process of gradual change on a branch is the best idea. | 13:02.43 |
sebras | Robin_Watts: I agree, but e.g. glib/gstreamer have .h per .c, so it's not uncommon. | 13:02.46 |
Robin_Watts | cos we may change our impressions as we go. Ultimately the degree of splitting is an aesthetic judgement. | 13:03.12 |
tor8 | Robin_Watts: things will move around.. that will make git rebase and merge painful | 13:03.15 |
sebras | tor8: it seems to me that this might mean we need to update the docs as well..? | 13:03.19 |
tor8 | sebras: that's because glib wants to be c++ ;) | 13:03.56 |
sebras | tor8: good point. | 13:04.06 |
tor8 | Robin_Watts: step 1 should be done and pushed to master IMO | 13:04.42 |
Robin_Watts | tor8: eh? make a branch, commit lots of small changes to it. When we're happy, we merge it. I don't see this taking more than a few days, so the degree of rebasing etc required should be small, I'd have hoped. | 13:04.46 |
tor8 | Robin_Watts: sure. just let's avoid changing header file stuff in the next few days then :) | 13:05.09 |
Robin_Watts | tor8: OK :) | 13:05.17 |
tor8 | and then we can tackle moving source files and updating the build systems as well | 13:05.42 |
Robin_Watts | right. | 13:05.47 |
sebras | tor8: when you move headers/create new ones, don't forget the .deb packaging. | 13:05.54 |
tor8 | both of these changes will wreak havoc on clients, so maybe push them both at once to master | 13:06.10 |
sebras | tor8: will you do a release with the current state now before you do these changes? | 13:06.38 |
tor8 | sebras: you mean release-release? | 13:06.51 |
| 1.3 and all that stuff? | 13:06.57 |
sebras | tor8: yes... a 1.3 or something. | 13:07.00 |
| or 1.2.5 or whatever. | 13:07.08 |
tor8 | I don't know, I think the next release isn't scheduled until august | 13:07.21 |
| which gives us plenty of time to rejig and become stable again :) | 13:07.34 |
sebras | wast libopenjpeg 2.0 merged in the 1.2 release? | 13:07.51 |
Robin_Watts | sebras: Gawd no. Releases are a pain in the ass. | 13:08.01 |
| sebras: No. 2.0 was post 1.2 | 13:08.08 |
tor8 | openjpeg2 just got merged in a few weeks ago | 13:08.12 |
| or days, even | 13:08.15 |
| I've got lcms2 added to the build system on a branch too | 13:08.38 |
sebras | Robin_Watts: obviously you need a release time that does the releases for you... ;) | 13:08.45 |
tor8 | maybe michael will have that done in time for august release, maybe not | 13:08.55 |
sebras | alright, then 1.3 seems to be a major upgrade anyway I guess. | 13:09.06 |
Robin_Watts | sebras: I have a tor8 to do releases :) | 13:09.52 |
henrys | I hate ghostscript parameters, get is write and put is read and the code reading nightmare has just begun. | 13:10.09 |
Robin_Watts | "Bad is good, and to shake ones booty means to wiggle ones butt." - Homer Simpson | 13:11.12 |
kens | sympathises | 13:11.16 |
| <sigh> Last weeek I was irritated because my number of open bugs had gone over 80 again, today I have 110 :-( | 13:13.45 |
henrys | kens:are all of alex bugs reassigned? I did a search didn't see any. | 13:15.29 |
Robin_Watts | There are still alex bugs out there. | 13:15.42 |
| tor8: In the begin_page/end_page commit, there is still the question of the rect being transformed in cases when it doesn't need to be. | 13:17.01 |
henrys | where he is the assignee? | 13:17.03 |
| I don't show any in a search | 13:17.13 |
Robin_Watts | And the //XXX line should be tested. | 13:17.25 |
| henrys: oh, sorry, the one I was looking at is now closed. | 13:17.37 |
kens | henrys I just finished reassigning them, yes | 13:17.46 |
Robin_Watts | oh, it's gone in already :( | 13:19.38 |
| tor8: The strain_soup one... does strain_soup leave the soup bowl empty? i.e. can we safely start a new page? If not, then the freeing of the soup should be in end_page | 13:22.29 |
tor8 | Robin_Watts: sorry, got a bit over-eager there | 13:23.10 |
| I am trying different approaches to the rect transform now | 13:23.25 |
Robin_Watts | tor8: no worries. | 13:23.31 |
tor8 | separate switches is ugly | 13:23.37 |
Robin_Watts | It looks to me like strain_soup leaves the soup bowl full. | 13:23.45 |
tor8 | there are 7 instances of transformed rects in the playback | 13:24.00 |
Robin_Watts | Have a 'needs_rect[type]' ? | 13:24.28 |
tor8 | Robin_Watts: the strain_soup state is what prompted me to do the single-page/multi-page check in a commit on wip | 13:24.32 |
| I think it'd be better to empty the soup bowl at end page and maybe append a new text_page on end_page? | 13:24.58 |
| i.e. make the text_page a linked list as well | 13:25.05 |
Robin_Watts | tor8: at the moment I'm just concerned that the soup bowl isn't emptied. | 13:25.24 |
| and appending a new text_page on end_page would be bad, as for a single page device, we don't want a second page appended. | 13:25.53 |
| We should append on a begin_page if we've seen an end_page. | 13:26.17 |
| oops. cluster compile fail. | 13:27.16 |
| The makefile cleanups have upset it. | 13:27.29 |
tor8 | I meant append on new_page | 13:28.36 |
Robin_Watts | right. | 13:28.43 |
tor8 | rats! | 13:28.45 |
| can you see what broke? | 13:29.16 |
Robin_Watts | tor8: something to do with the v8 lib, I guess. | 13:30.03 |
tor8 | Robin_Watts: ah! | 13:31.22 |
| I think I have it | 13:31.25 |
Robin_Watts | ok. | 13:31.29 |
tor8 | it's linking c++ with c linker | 13:31.37 |
| linking with $(CXX) implicitly adds -lstdc++ (or whatever the c++ runtime lib is called) | 13:32.38 |
| V8_LIBS should add -lstdc++ and then it ought to work | 13:33.03 |
| Robin_Watts: try the last commit on tor/master if that helps the cluster compile | 13:34.56 |
Robin_Watts | I've squashed that into the original one, and I'm cluster testing now. | 13:43.44 |
| if it passes I'll force push and we'll all pretend it didn't happen. | 13:44.00 |
tor8 | Robin_Watts: if you force push, drop the begin/end page ones and we can squash fixes into them as well | 13:45.23 |
Robin_Watts | ok. | 13:45.34 |
tor8 | (since they're later in the history anyway) | 13:45.44 |
Robin_Watts | yeah. makes sense. | 13:47.06 |
tor8 | transform and span-soup fixes on tor/master now | 13:47.40 |
| untested text extraction though | 13:48.11 |
| oh bollocks, it doesn't work :( | 13:48.38 |
Robin_Watts | I've backtracked golden to before the broken commit. | 13:49.43 |
| I'm going to grab some lunch, so take your time :) | 13:49.56 |
tor8 | Robin_Watts: about the rect transform ... we're already transforming every one to check for visibility | 14:09.35 |
Robin_Watts | tor8: Then why do we transform a second time ? | 14:28.12 |
tor8 | we forgot to remove those transforms when adding visibility tests? | 14:28.40 |
| new and updated commits on tor/master | 14:28.59 |
Robin_Watts | We should never elide begin or end page based on the rect though, right? | 14:30.32 |
tor8 | Robin_Watts: yeah. that'd be bad. | 14:30.45 |
| I tested the XXX | 14:31.19 |
| no differences (as expected, since the ctm is probably fz_identity in almost all of the cases we use) | 14:31.47 |
| but looking at the code, it really ought to be transformed | 14:32.06 |
Robin_Watts | For 200dpi, the list will be made with the identity, but played back at 200dpi. | 14:32.32 |
| so that will test this code. | 14:32.43 |
| so if your tests show no diffs, that's a step forward. | 14:32.56 |
| ok, they all seem reasonable to me. | 14:35.25 |
tor8 | thanks. will you do the honor of breaking everything by pushing? :) | 14:36.58 |
Robin_Watts | I am just cluster pushing now (call me a coward :) ) | 14:37.20 |
| then I'll push etc. | 14:37.25 |
tor8 | thanks :) | 14:37.34 |
Robin_Watts | and fight the cluster until it properly tests stuff. | 14:37.36 |
tor8 | Robin_Watts: did you see the link I sent you last night about SDF text rendering? | 14:38.37 |
Robin_Watts | I saw you'd sent it, and meant to read it this morning. but my PC rebooted during the night thanks to windows updates. | 14:39.12 |
| and wouldn't come up with a working mouse this morning. I've had to disconnect the mouse from the KVM for some reason. | 14:39.31 |
| The NVidia path rendering extension appears to work by being passed 'text strings' of paths. | 14:42.09 |
| like "M 100 100 L 200 100 C 300 400 500 600 700 800 Z" etc | 14:42.29 |
| Which means that instead of rendering paths ourselves, we'd have to convert paths to strings, pass the strings to the NVidia stuff, which would then bake it into texture memory. | 14:44.14 |
| And then it could render them. | 14:44.24 |
| The NVidia stuff (as far as path rendering goes) looks to be a win if you repeatedly rerender the same stuff again and again (possibly at different zoom levels) | 14:44.59 |
tor8 | http://developer.download.nvidia.com/assets/gamedev/files/GL_NV_path_rendering.txt | 14:45.08 |
| that's the official spec for the extension | 14:45.14 |
dogisfat | I do not know if this is the right place to ask this but I have been working with using GS to interact with PDFs and found that if a bitmap exists in a PDF GS will render any paths (flatten) any paths present in the document. | 14:45.17 |
tor8 | so there are a dozen ways to feed it a path. one of them is via strings. | 14:45.24 |
Robin_Watts | dogisfat: You're talking about using gs with the pdfwrite output file, right? | 14:45.55 |
kens | dogisfat, I don't thin this is the case | 14:46.03 |
tor8 | give it a path, you then call glStencilFillPath() | 14:46.10 |
| and that'll fill out the stencil buffer for the covered area | 14:46.21 |
Robin_Watts | tor8: but ultimately we always have to pass paths in and it will convert to texture space data. | 14:46.30 |
tor8 | then using that stencil buffer, glCoverFillPath will draw colors masked by the stencil buffer | 14:46.42 |
Robin_Watts | Right. So we'd always be using BAKE, then STENCIL, then COVER. | 14:46.45 |
dogisfat | Robin_Watts: I am using GS to convert PDFs to other formats like EPS or PS | 14:46.45 |
tor8 | it doesn't convert to texture data | 14:46.55 |
| it renders directly to the stencil buffer | 14:47.04 |
kens | dogisfat, you are probably using pswrite, don't. Use ps2write | 14:47.09 |
Robin_Watts | It renders from 'path data in its own form' to the stencil buffer. | 14:47.19 |
tor8 | we can do the same with trapeziods to emulate the same behaviour | 14:47.20 |
| what you gain by the extension is not having to do the scan conversion and tesselation into triangles | 14:47.45 |
dogisfat | kens: I was using epswrite | 14:47.58 |
Robin_Watts | and the 'path data in it's own form' is what it calls 'baked' data. Which is held in graphics ram (texture memory). | 14:48.00 |
kens | dogisfat, epswrite is just pswrite with a different name | 14:48.12 |
Robin_Watts | For high res stuff, where pixels massively outnumber paths, I can see that it's probably a win. | 14:49.01 |
| For low res stuff, where the complexity of the paths are the deciding factor, I think it's less obviously a win. | 14:49.28 |
kens | dogisfat, pswrite/epswrite convert *lots* of stuiff to bitmaps, I wouldn not use either | 14:49.39 |
Robin_Watts | but the potential to do the whole lot (blending and all) in the GPU probably makes up for it. | 14:49.53 |
| i.e. the speed of the path rendering is less important - it's just important that it's within the GPU. | 14:50.12 |
tor8 | Robin_Watts: reducing the number of draw calls (batches) is always a win, to reduce driver overhead | 14:50.35 |
| on mobile devices, there's usually shared system/gpu memory so having data reside on the "gpu" side of things is not as important as on desktop | 14:51.10 |
Robin_Watts | tor8: compile failure :( | 14:51.21 |
tor8 | for textures, EGLImage is the fastest way to get data across the bus. you can tell it to use our system memory pixmaps, and not do any data format conversion at "load" time | 14:52.01 |
dogisfat | kens: ps2write produces a flattened result as well | 14:52.08 |
tor8 | so the gpu will directly access our unpacked pixmaps | 14:52.15 |
Robin_Watts | tor8: right, but there is still non-trivial conversion of path data. | 14:52.17 |
| tor8: but getting pixels back is hard. | 14:52.33 |
kens | dogisfat, if this is the case then you need to report a bug. HOwever, be aware that this is probably due to the original containing *tarnsparency* , not a bitmap. PostScript does not support transparency and so it must be rendered to an image | 14:53.08 |
tor8 | paths are less of a problem. it's "solved" by the nv_path_rendering extension for a sufficiently small subset of "solved" :) | 14:53.32 |
| we could reimplement the same algorithms as used in the extension, and not rely on nvidias extension, but possibly suffer worse performance | 14:54.02 |
dogisfat | kens: so if there is transparency in an image within the PDF GS will flatten the entire thing? | 14:54.28 |
kens | just the transparent bits | 14:54.40 |
tor8 | we'd scan convert the path into a triangle mesh. the mesh data would be stored in a vertex buffer object on the gpu side. | 14:54.48 |
Robin_Watts | tor8: The way I see it is that the NVidia path rendering extension (or if not the extension, at least the algorithm) is the last piece in the puzzle that was stopping us doing an entirely hardware rendering pipeline. | 14:54.54 |
| but I don't think the path rendering stuff alone is going to get us a mammoth speedup. | 14:55.14 |
kens | gogisfat, be aware that Cairo-produced PDF files tend to declare the whole page as transparent | 14:55.28 |
dogisfat | kens: so if the paths don't intersect a transparent region they should not be flattend | 14:55.43 |
tor8 | shuffling clip and transparency group states and temporary buffers is the big problem for performance | 14:55.49 |
| swapping render targets mid-frame is sloooow | 14:55.57 |
kens | dogisfat, yes but see my comment above | 14:56.11 |
Robin_Watts | tor8: So you'd not follow the same stencil approach that they take? | 14:56.12 |
tor8 | Robin_Watts: my current thinking is to use stencils for all path rendering, like they do | 14:56.35 |
| and also for clipping. it all falls out naturally. | 14:56.49 |
| but it puts a hard limit on the number of clips you can nest | 14:57.01 |
| given that we don't need a depth buffer, a max clip depth of 31 with a 32-bit deep stencil buffer ought to do the job | 14:57.23 |
| and rectangular clips can be done by simple scissoring | 14:57.34 |
| soft masks and transparency groups will be slow, since they involve creating temporary textures that we have to set as the render target to draw into, and then bind as textures to read from | 14:58.18 |
| but hopefully they can be cached between renders :) | 14:58.28 |
Robin_Watts | tor8: Can't we do better than that ? (for clipping) | 14:58.55 |
tor8 | (max depth of 31, because drawing paths will use one bit plane of the stencil) | 14:59.12 |
Robin_Watts | I think the nvidia stuff uses 8 bits of stencil mask. | 15:00.00 |
tor8 | Robin_Watts: not that I can think of. we could draw masks to textures and use those, but that's going to be slower than we want. much slower. | 15:00.07 |
Robin_Watts | then converts that to 1 bit (effectively) before combining masks. | 15:00.17 |
| I'll confess to being a tad out of my depth here w.r.t. opengl internals, so I'll bow to your greater knowledge. | 15:00.56 |
| tor8: And we just got a mail back from NVidia. | 15:01.10 |
| I'm confused as to how mujstest has ever linked right since I changed the v8 libs. | 15:01.50 |
tor8 | Robin_Watts: I still need to follow up with Nikolay about a date and time. | 15:02.55 |
Robin_Watts | ok. | 15:03.01 |
dogisfat | What is the best way to test a PDF for transparent content? | 15:03.04 |
tor8 | shall I tell him tomorrow at 15.00 CET? | 15:03.21 |
Robin_Watts | dogisfat: Hold it up to the light and see if you can see through. | 15:03.22 |
| Sure. | 15:03.27 |
tor8 | 14.00 your time | 15:03.28 |
Robin_Watts | I can be ready at any time. | 15:03.38 |
kens | dogisfat I believe pdfinfo.ps will give you this information | 15:04.07 |
marcosw | tor8 and Robin_Watts: there was something wrong with the mupdf regression repository; I blew it away and did a new git clone. | 15:04.35 |
Robin_Watts | marcosw: Yes. I was fiddling with it. | 15:04.46 |
dogisfat | Robin_Watts, Haha. kens, Thank you, I will look into it. | 15:04.54 |
marcosw | Robin_Watts: sorry, did I mess up your work? | 15:05.08 |
Robin_Watts | marcosw: S'OK. I backtracked history to erase some broken commits. | 15:05.34 |
| So no harm done. | 15:05.39 |
marcosw | I was getting lots of "fatal: Refusing to fetch into current branch refs/heads/incoming_master of non-bare repository" messages. | 15:06.09 |
tor8 | Robin_Watts: it's a matter of how deep stencil buffers the drivers support. at least 8 bits on tegra, so that could be a potential problem. | 15:06.11 |
Robin_Watts | tor8: Have you read their paper? They talk about their use of stencil buffers. | 15:06.40 |
| and they get away with 8 bits. | 15:06.45 |
tor8 | Robin_Watts: I will need to study it in more depth (I jumped straight to the extension spec...) | 15:07.20 |
| but I'm talking about using the stencil buffer for all clip masking | 15:07.37 |
Robin_Watts | Ah. I read the algorithm rather than the spec. | 15:07.41 |
tor8 | so we have to support gsave/grestore | 15:07.44 |
Robin_Watts | right. THat's what they do. | 15:07.49 |
tor8 | okay. I'll read it some more. there may be some tips and tricks I haven't thought of. | 15:08.05 |
| the nexus 10 uses an arm mali chip | 15:08.14 |
Robin_Watts | This is still not working on the cluster. I'm going to have to try building on peeves myself. | 15:08.28 |
| mvrhel_laptop: Morning. | 15:16.20 |
| Looking at your reviews is on my list of things to do, just after solving some build problems we've got. | 15:16.43 |
sebras | tor8: so does several other cellphone vendors. | 15:18.33 |
| so does? so do! | 15:18.50 |
Robin_Watts | paulgardiner: ping ? | 15:25.18 |
paulgardiner | hi | 15:25.35 |
Robin_Watts | On android, the fuzzy redraw is done from a cached thumbnail, right? | 15:25.57 |
| And that's rendered on another thread? | 15:26.20 |
paulgardiner | Not cached any longer than the lifetime of the view | 15:27.06 |
Robin_Watts | paulgardiner: Right. When we go to a page we do a fuzzy redraw and keep that until we move more than 1 page away. | 15:28.08 |
paulgardiner | Yes | 15:28.17 |
Robin_Watts | But that fuzzy redraw happens on a different thread? | 15:28.20 |
| Just trying to reply to NVidia about multithreading. | 15:28.41 |
paulgardiner | Everything that involves use of the library happens on thread other than the UI thread | 15:28.48 |
Robin_Watts | paulgardiner: But do we ever have 2 redraws going on at once? | 15:29.00 |
| fuzzy and hq simultaneously. | 15:29.11 |
paulgardiner | No because MuPDFCore has all synchronised methods | 15:29.34 |
Robin_Watts | ok. | 15:29.56 |
paulgardiner | Although we use many threads, it is as though we have two, one for the UI and one for library interactions | 15:30.22 |
| Presumably, if we were using their tech, we would be doing the baking at the time at which we now do the render, and the actual render would happen in Android's own OGL render loop | 15:32.31 |
Robin_Watts | I'm confused. | 15:45.03 |
| If I backtrack to 8b3bfb0 and cluster push that, it fails. | 15:45.27 |
paulgardiner | Did you cluster test it while we were at the meeting? | 15:47.07 |
Robin_Watts | paulgardiner: According to the regression tests it passed. | 15:48.33 |
ray_laptop | mvrhel_laptop: I had a question about gsicc_mcm_begin_monitor -- when you have a chance | 15:48.44 |
| mvrhel_laptop: (actually the question relates to mcm_end_monitor, but that was what I cloned begin_monitor from) | 15:49.22 |
paulgardiner | Robin_Watts: possibly worth trying: hard reset to 8b3bfb0; soft reset to golden/master; commit -a and then cluster test | 15:50.42 |
| ... assuming you have no important diffs in your working copy or staging area that is. | 15:51.32 |
Robin_Watts | I did: git checkout 8b3bfb0 && git cluster mujstest | 15:51.59 |
| git diff and git diff --cached both show empty. | 15:52.20 |
mvrhel_laptop | hi ray_laptop | 15:52.31 |
Robin_Watts | On peeves at least, thirdparty/v8-3.9 has only libv8_shapshot.a, no libv8_base.a in. | 15:53.04 |
paulgardiner | Robin_Watts: I was thinking that the cluster may be having trouble with testing an old point in the tree, but might be okay with the working copy at that point added as a new commit on the end of the tree | 15:54.51 |
henrys | ray_laptop:do you know the history of Duplex_set - why do we need it all the printer devices I know of would simply ignore duplexing commands if they aren't able? | 15:55.04 |
Robin_Watts | paulgardiner: I can't see why. | 15:55.22 |
henrys | s/able to duple | 15:55.24 |
| s/able/able to duplex | 15:55.36 |
paulgardiner | Robin_Watts: nor me, but that test would rule it out | 15:55.45 |
ray_laptop | henrys: Duplex (and Duplex_set) are part of the "standard" params documented in the PLRM | 15:57.14 |
henrys | Oh I thought Duplex_set was our doing, thanks | 15:57.43 |
ray_laptop | henrys: Duplex_set is not, but is used internally to tell us whether or not Duplex was set, or is just defaulting | 15:57.57 |
| henrys: so you are right, Duplex_set is ours | 15:58.31 |
Robin_Watts | paulgardiner: I'm running it again as a git job rather than a user one in case it's just user jobs that are broken., | 15:58.52 |
| yeah, looks like it's just user jobs that are broken. in fact, possibly just for me. | 15:59.37 |
| paulgardiner: Could you try a user run of mujstest for 8b3bfb0 please? | 15:59.57 |
ray_laptop | henrys: the PLRM says that Duplex is an optional parameter, and if the device doesn't support Duplex, we should throw an error (configurationerror), but that messes up rendering PS files that were captured from a driver that sets duplexing | 16:00.09 |
henrys | I don't see why Duplex_set is needed at all and it gets quite messy with command line options and all, ray_laptop - if the device wants to throw an error when Duplex is specified it can do so. | 16:00.57 |
| In particular it isn't clear to device writer where to set the option - some have set it int the open device call but that is too late for -dDuplex on the command line. | 16:02.00 |
deleet | you guys do some jni, do you know how I can get a jni callback in java on a non-ui thread? | 16:02.07 |
ray_laptop | henrys: it's been a while, and I don't recall the reason for Duplex_set. Looking at gdevdjet.c it seems to use it | 16:02.18 |
Robin_Watts | deleet: You want to cause C to call back to java? | 16:02.32 |
deleet | yes. it already does but all the callbacks end up on the ui thread, which considering the work they have to do (collation) is not good | 16:02.58 |
Robin_Watts | AFAIK, we've never done that. What we HAVE done is to start a new thread in java, and have that call into C, where it sleeps. | 16:03.15 |
henrys | ray_laptop:I can fix my current bug using it, it just struck me as yet another level of confusion for the parameter code. | 16:03.29 |
Robin_Watts | Then other C threads can wake it up, whereupon it will return to the java and do stuff, then call back into the C and sleep again. | 16:03.38 |
deleet | yeah but since apparently you can only call static java methods, I get ui thread :/ | 16:03.40 |
henrys | did I mention I hate the parameter code. | 16:04.05 |
Robin_Watts | hence we get the effect of the C stuff calling java. | 16:04.12 |
| on a non UI thread. | 16:04.17 |
deleet | hmmm. | 16:04.30 |
paulgardiner | Robin_Watts: job queued | 16:04.32 |
Robin_Watts | Thanks. | 16:04.41 |
deleet | Robin_Watts: I'll play around with that concept, thanks for the hint | 16:04.47 |
Robin_Watts | see "alertsInit" in the mupdf code, I think. | 16:05.14 |
ray_laptop | henrys: Duplex_set < 0 implies "the device doesn't support Duplex", so a device can enable/disable this without having to code a get/put params | 16:06.30 |
paulgardiner | Looks like golden/master is 8b3bfb0. so when you said backtrack, you meant down your own branch | 16:06.44 |
Robin_Watts | paulgardiner: golden master was for a while up beyond 8b3bfb0. | 16:07.15 |
ray_laptop | henrys: but by default in the macros that initialize devices, Duplex_set is 0, so it is enabled. | 16:07.18 |
Robin_Watts | I've forced us back to that. | 16:07.22 |
henrys | ray_laptop:Duplex could just be handled by the printer superclass the device doesn't need to do anything if we remove Duplex_set | 16:08.12 |
| ray_laptop:there is -1 0 and greater for duplex_set - see get and put params in the gdevprn.c | 16:09.37 |
ray_laptop | henrys: if a device has duplex_set == -1 then it looks like the Duplex param is disabled. | 16:19.58 |
| henrys: i.e, in gdev_prn_get_params, Duplex won't get returned if ppdev->Duplex_set < 0 | 16:21.20 |
henrys | ray_laptop:I know | 16:21.47 |
ray_laptop | henrys: and by default, it looks like the macros invoke prn_device_body_rest2_ with duplex_set == -1 | 16:23.11 |
| henrys: then I don't understand your assertion: Duplex could just be handled by the printer superclass the device doesn't need to do anything if we remove Duplex_set | 16:24.38 |
Robin_Watts | tor8: OK, pushed. Hopefully this should work. | 16:24.45 |
henrys | yeah I've grokked all the code and immediately wanted to know why we need this. Just set it, if you send any printer device wants to use the duplex command it can. | 16:25.05 |
ray_laptop | henrys: because Duplex_set is _how_ the printer superclass determines if the particular instance of the device supports duplexing | 16:25.21 |
henrys | rewording | 16:25.21 |
| it doesn't matter if it supports duplexing. All these devices can be sent duplexing command and they will ignore them. | 16:26.00 |
ray_laptop | henrys: well, it is contrary to the Adobe PLRM for a device that doesn't support duplexing to have the Duplex command in the param list. | 16:26.45 |
henrys | oh well I didn't know that. | 16:27.13 |
ray_laptop | i.e., a PS program can do something different if the device doesn't return /Duplex in the dict returned by currentpagedevice | 16:27.34 |
henrys | okay that explains it then. | 16:28.05 |
| I thought the key would be there but it would just be false. | 16:28.26 |
kens | henrys, ray_laptop,chrisl. I've been looking at bug #226943 'THere's no %ram% device. We've beensupplied 2 patches, one for a 'ram' device that actually uses disk files and one for a 'real' ram device. I'm not an expert in this are, but I quite like the look of the real ram device. Do you all think its worth going ahead with this / I'd have to try and contact the author again, which might be difficutl after 7 years...... | 16:28.41 |
ray_laptop | henrys: but it does mean that a PS file created for a duplexing device that doesn't properly catch a configurationerror from setpagedevice might not work | 16:29.17 |
| kens: yes, I agree that the "real" ram device is better. We'll probably need a CLA at the least | 16:30.02 |
| henrys: but such a badly written PS file is _supposed_ to fail according to the spec | 16:30.32 |
kens | ray_laptop : yes that was my thinking too. I wanted a little consensu that this is useful enough before going ahead. I'll need to compile and test it as well | 16:30.42 |
ray_laptop | but then, Adobe Distiller may not -- I haven't tried it | 16:30.58 |
kens | However, it looks like it conforms well tothe GS idioms | 16:31.04 |
ray_laptop | kens: right. I looked at this a bit between fires, but never completed reviewing and testing it | 16:31.39 |
| kens: but I agree that it looked decent | 16:31.54 |
kens | ray_laptop : I saw your comments in the thread :-) | 16:31.54 |
| OK so tomorrow I'll compile and test it, then try and contact the authour. | 16:32.10 |
ray_laptop | kens: yours now to do what you want :-) | 16:32.21 |
kens | ray_laptop : Hmmm, I still like to consult on areas I don't really feel comfortable with, and this is one of them. :-) | 16:32.42 |
ray_laptop | I have to run something to the school. The last time this term :-) | 16:32.55 |
kens | That's only because term is nearly over :-) | 16:33.09 |
ray_laptop | kens: if you have specific questions, I'll be glad to consult | 16:33.21 |
chrisl | kens: I agree, we should have a real ram device - I'm a little surprised we haven't have more complaints about it | 16:33.31 |
kens | ray no problem, I'll give it a go tomorrow | 16:33.31 |
ray_laptop | kens: right. last day for one, and tomorrow is 1/2 day for the other 2 | 16:33.43 |
Robin_Watts | mvrhel_laptop: ping | 16:33.50 |
ray_laptop | kens: g'nite, kens | 16:33.52 |
kens | bye ray_laptop | 16:34.01 |
ray_laptop | mvrhel_laptop: ping 2 (please ring me up) | 16:34.08 |
mvrhel_laptop | Robin_Watts: ray_laptop: pong | 16:34.40 |
Robin_Watts | mvrhel_laptop: I have some comments on your review, but it can wait if you want to call ray first. | 16:34.58 |
mvrhel_laptop | Robin_Watts: ok I will do that first | 16:35.11 |
kens | night all | 16:35.15 |
henrys | chrisl:did you hear back from russell | 16:41.43 |
| ? | 16:41.44 |
chrisl | henrys: no I haven't heard anything more. I did pull the entire (remaining) wisc pages onto casper this afternoon | 16:47.29 |
mvrhel_laptop | Robin_Watts: ok I am ready | 16:50.43 |
Robin_Watts | ok, so second commit looks fine. | 16:51.07 |
| first commit I have some comments on | 16:51.13 |
| In ScrollChanged, you have some repeated code. | 16:51.29 |
| The code seems to do: | 16:51.34 |
| if (this isn't the page we have a display list for) { make the list; render the list; } else {render the list;} | 16:52.15 |
| Why not: | 16:52.21 |
| if (this isn't the page we have a display list for) { make the list; } render the list; | 16:52.56 |
| Today, I shall be mostly trying to type lust instead of list. Freudian? | 16:53.11 |
mvrhel_laptop | Robin_Watts: it has to do with the way that the async calls are dependent upon one another | 16:53.13 |
| I had wanted to do it that way | 16:53.20 |
Robin_Watts | mvrhel_laptop: I thought that might be the case. | 16:53.27 |
mvrhel_laptop | but the create task and the continuation makes it hard | 16:53.32 |
Robin_Watts | Is there no way to have the first async call be to something that contains the if ? | 16:53.56 |
| i.e. have async ( "if (we have the display list for the wrong page) make it for the right one"); then("render the display list;") | 16:54.38 |
mvrhel_laptop | if I had the CreateDisplayList do a quick return if the page is already set, I think I could do it then | 16:55.03 |
| oh you had the same thought | 16:55.17 |
Robin_Watts | I also wondered why you didn't push the display list making into RenderPage ? | 16:55.18 |
mvrhel_laptop | Robin_Watts: I don't always want a display list, and I dont want the background thumbnails for example creating display lists. I thought this would be better | 16:56.14 |
Robin_Watts | OK, you may be right. | 16:56.22 |
| These were idle thoughts only - I'll confess to only vaguely following all the continuation gubbins. | 16:56.57 |
| Then in muctx.... | 16:57.09 |
| you free the display list, which uses the context, after you have freed the context. | 16:57.24 |
mvrhel_laptop | Robin_Watts: I can likely reduce the if else task continuation stuff | 16:57.27 |
| Robin_Watts: oh that is not good | 16:57.36 |
| how did that work | 16:57.53 |
Robin_Watts | Also, there still looks to be if (x != null) free(x); type checks in there. | 16:58.03 |
| mvrhel_laptop: Well, as long as we don't blank the context structure on destruction, you'll get away with it. | 16:58.19 |
| (i.e. a memento run would kill it) | 16:58.28 |
mvrhel_laptop | ok. let me get get these cleaned up | 16:58.47 |
Robin_Watts | mvrhel_laptop: Hold on. | 16:59.01 |
| Jumping back to the previous thing. | 16:59.06 |
mvrhel_laptop | I am not sure how to merge these changes into a commit that is not at the head | 16:59.20 |
Robin_Watts | about "why didn't you do the creation/use of the display list in RenderPage"... | 16:59.40 |
| RenderPage takes a "use_dlist" argument. | 16:59.49 |
| so I don't understand your objection. | 17:00.11 |
| mvrhel_laptop: merging the changes into a commit that is not the head is easy. | 17:00.32 |
| You make the changes, and commit them to a new commit. | 17:00.38 |
| Then you do an interactive rebase (rebase -i HEAD~10 at the command line, or however tortoise does it) | 17:01.11 |
| and you move the last line (your new commit) to be just after the one that it wants to squash into. And you change it from "pick" to "s". | 17:01.45 |
henrys | trivial ;-) | 17:02.09 |
Robin_Watts | henrys: How could it be any simpler? | 17:02.26 |
chrisl | Got to go - nite folks! | 17:02.43 |
Robin_Watts | He has some changes that he wants to squash into an earlier commit. | 17:02.47 |
| so you package up the changes, and tell git to squash them into the earlier one. | 17:03.05 |
henrys | oh I see what you are saying - I misunderstood. | 17:03.36 |
mvrhel_laptop | Robin_Watts: ok my thought on keeping a render page and a display list separate are to maintain some flexibility for myself at the higher level. With thumbnails, I simply want to get the bitmap. When I am in a zooming situation I do want a display list created. I suppose I could put more complexity in the Renderpage to do this, but I had at the time made the decision to let that be set... | 17:03.47 |
| ...by the application. For example, I may want to only create and cache display lists for a few pages around where I am but not render them yet. I am not doing that, but the thought occurred to me | 17:03.48 |
Robin_Watts | mvrhel_laptop: Right, but would you not just pass use_dlist = 1 for full pages, and use_dlist = 0 for thumbnails ? | 17:04.24 |
| It's not important, it's just an idle observation. | 17:04.49 |
mvrhel_laptop | Robin_Watts: to me the use_dlist = 1 was not to create a display list, but to use the display list | 17:06.39 |
| that had already been created | 17:07.01 |
Robin_Watts | mvrhel_laptop: Right. And if we pushed the creation in there, then use_dlist would mean "create a dlist if you don't have one, and use it". | 17:07.29 |
| So the application would still be in control as to whether a given render used a dlist or not. | 17:07.52 |
mvrhel_laptop | Robin_Watts: but there may be cases where I dont want to render it | 17:07.55 |
Robin_Watts | It's just the app wouldn't *have* to handle the dlist creation itself. | 17:08.17 |
mvrhel_laptop | I guess I could add another option to create list but not render it | 17:08.19 |
Robin_Watts | really? | 17:08.24 |
mvrhel_laptop | really what? | 17:08.32 |
Robin_Watts | why would you want to create a dlist and not render it? | 17:08.35 |
| I don't think that's a use case we've ever met in any of our apps. | 17:09.11 |
| (but I could be being myopic here) | 17:09.19 |
mvrhel_laptop | Robin_Watts: well, if I were to create display lists for a small +/- page region around where I currently am and cache those, it could potentially be helpful | 17:10.07 |
Robin_Watts | and that caching won't work at the RenderPage level? Fair enough. | 17:10.34 |
mvrhel_laptop | I would not necc. want to render those until I go to those pages | 17:10.36 |
| anyway that was my thinking | 17:10.57 |
Robin_Watts | OK, like I say, I'm sticking my nose into stuff that I don't 100% understand, so please just ignore me. | 17:11.09 |
mvrhel_laptop | If I were writing an app with the library, I would prefer to have the flexibility to create caches of display lists and/or rendered pages | 17:11.32 |
| if I so desired | 17:11.40 |
Robin_Watts | fair enough. | 17:11.43 |
| I'll shut up and let you make the changes then. Give me a yell if you need a hand with the git stuff. | 17:12.04 |
| (you could do a separate commit and I'll do the squashing if that helps) | 17:12.25 |
mvrhel_laptop | Robin_Watts: ok. let me read what you wrote about git. I am sure I will need som help | 17:12.28 |
| ok. so I can first just do a new commit | 17:12.59 |
Robin_Watts | yes. | 17:13.08 |
mvrhel_laptop | and worry about the rest later down the road | 17:13.14 |
Robin_Watts | yeah, however we do this, the first step is to do a new commit. | 17:13.32 |
| tor8: begin/end page caused diffs in the cluster. | 17:14.01 |
| I will attempt to figure out why. | 17:14.12 |
| henrys: sorry if I sounded all defensive there. It's just that with SVN etc, you just couldn't do any of this stuff. It's the power of git that makes it possible. And then people complain that git is hard :) | 17:21.53 |
henrys | rebase -i is a killer git feature IMHO ⦠I misunderstood what was going on. | 17:22.55 |
Robin_Watts | yeah, it gives us the opportunity to rewrite history. We can go back and make it look as if we didn't make any wrong steps while we develop. | 17:24.16 |
| henrys: What's a typical printer controller cost? $80 ? | 17:30.15 |
henrys | Robin_Watts:I haven't looked recently that sounds right if not a bit high. | 17:33.11 |
Robin_Watts | henrys: Have you read the email chain today? Sounds like they might be in the right ballpark. | 17:35.15 |
henrys | Typically these graphics accelerators are optimized for a screen experience, printers are all about hi res halftoning - at least the specialized hardware we see on printers is dedicated to this problem. | 17:38.30 |
Robin_Watts | henrys: To do transparency etc fast, you need to be working in contone. | 17:39.16 |
| and the beauty of the programmability of shaders is that we probably get to work in contone all the way through, and then still get to halftones fast. | 17:39.48 |
| typical printer controller = RAM + CPU + ASIC for halftoning. | 17:41.41 |
| that's not much different from RAM + CPU + GPU combo, if the GPU can be driven in the right way. | 17:42.11 |
| tor8: I'm confused. | 17:42.53 |
| The cluster report for b975f1b (begin/end stuff) shows 204 diffs. | 17:43.17 |
| inn the mupdf run. | 17:43.32 |
| but backtracking to the one before that and pushing a cluster job doesn't show any differences. | 17:43.50 |
tor8 | my sane tests didn't show any differences | 17:44.11 |
| Robin_Watts: cluster could be confused by the history rejig? | 17:44.45 |
Robin_Watts | The rejig started from 8b3bfb0, and this is 2 commits further on, so I'd hope not. | 17:45.37 |
mvrhel_laptop | Robin_Watts: ok I am going to commit these changes now | 17:45.40 |
Robin_Watts | mvrhel_laptop: ok. | 17:45.46 |
| tor8: Something very odd is up with the cluster | 17:53.50 |
| I run a job just before the offending commit; no changes from trunk. | 17:54.17 |
| I run the offending commitl no changes from trunk, 204 changes from previous clusterpush. | 17:54.36 |
| So something is lying. | 17:54.41 |
| I suspect we're falling foul of the "do any of the changes match previous versions" code that's in there to cope with indeterminisms. | 17:55.24 |
mvrhel_laptop | I was able to move my commit. Now trying to figure out how to squash it | 18:01.27 |
| hmm I may have mucked up something | 18:03.42 |
Robin_Watts | How did you move it? By using an interactive rebase ? | 18:04.57 |
mvrhel_laptop | Robin_Watts: in tortoise git. That was pretty easy | 18:05.11 |
| The squash should work too. But it still shows the two commits in the log | 18:05.30 |
Robin_Watts | Are all your commits showing ? i.e you haven't lost any? | 18:05.49 |
mvrhel_laptop | I the commits are there | 18:05.57 |
| s/I/All | 18:06.02 |
Robin_Watts | It might be showing both the old and the new commits in the log, maybe? | 18:06.22 |
mvrhel_laptop | So I have 3 commits. The zoom in, the new commit, and the original display list commit | 18:06.25 |
Robin_Watts | OK. So do you want to try the squash again? | 18:06.49 |
mvrhel_laptop | Robin_Watts: yes. lets try it from ming | 18:06.58 |
Robin_Watts | mvrhel_laptop: Have you set up an editor you are happy with ? | 18:07.17 |
mvrhel_laptop | no | 18:07.20 |
Robin_Watts | Do you have an editor you are happy with in your path? | 18:08.09 |
mvrhel_laptop | now I am set | 18:08.55 |
| with notpad2 | 18:09.01 |
| so git rebase -i master | 18:09.10 |
| ? | 18:09.22 |
| oops editor not found | 18:09.49 |
| hold on | 18:09.56 |
Robin_Watts | That's not the right command. | 18:11.22 |
mvrhel_laptop | oh | 18:11.32 |
Robin_Watts | git rebase -i <earliest point you want to see in the list> | 18:11.42 |
| so: git rebase -i master~10 | 18:11.56 |
| that'll show you the last 10 things in the log. | 18:12.06 |
| assuming you're on master, that is. | 18:12.24 |
mvrhel_laptop | ok. hold on editor still not found. very odd since which notpad2 shows the path just fine | 18:12.26 |
| and yes I am on master | 18:12.31 |
Robin_Watts | mvrhel_laptop: At the msys prompt type "notpad2" and hit return. | 18:12.57 |
| Does that work ? | 18:13.06 |
mvrhel_laptop | oops. mispelled hold on | 18:13.19 |
| had export EDITOR=notpade | 18:13.40 |
| had export EDITOR=notpad2 | 18:13.44 |
| ok now I am in editor with ~10 | 18:14.01 |
Robin_Watts | ok. so the last 3 non-comment lines should be the most recent commits. | 18:14.19 |
mvrhel_laptop | so these are the three | 18:14.30 |
| pick f7c0b63 Addition of display list rendering in WinRT library | 18:14.35 |
| pick 960f1bd Clean up of a few items from commit f7c0b63a50b3d9bb1b16abb89e95ce80cf712a2e | 18:14.36 |
| pick f8845a0 Addition of zoom in and zoom out control on top app bar. | 18:14.38 |
Robin_Watts | You want to change the second of those from 'pick' to either 'f' or 's'. | 18:14.53 |
| 's' will give you the option of rewriting the commit messages. | 18:15.08 |
| 'f' will silently throw away the second commit message. | 18:15.19 |
mvrhel_laptop | ok f it is | 18:15.23 |
Robin_Watts | ok, make the change, save the file, close the window. | 18:15.39 |
mvrhel_laptop | ok. log looks good | 18:16.38 |
| Now I need to get my repos on casper fixed | 18:16.59 |
| do I just do a forced push | 18:17.11 |
Robin_Watts | push to origin master and tick the force box. | 18:17.21 |
| yeah. | 18:17.23 |
mvrhel_laptop | ok looks good | 18:17.59 |
| thanks Robin_Watts | 18:18.02 |
Robin_Watts | no worries. | 18:18.05 |
| gimme a mo, and I'll look 'em over and push them. | 18:18.20 |
mvrhel_laptop | Robin_Watts: ok. I am going to work on the tap moving stuff or do you think I should push what we have to the store (after having you test the package). | 18:19.25 |
| henrys: do you have an opinion about this? | 18:20.03 |
Robin_Watts | mvrhel_laptop: IME, pushing to stores takes time. | 18:20.55 |
| at least the first time. | 18:21.21 |
mvrhel_laptop | yes | 18:21.26 |
Robin_Watts | You probably need to generate a magic key and keep it safe somewhere. | 18:21.35 |
| I have the android one backed up in my homedir in casper with all the details etc in a text file called AndroidKeyDetails.txt | 18:22.02 |
mvrhel_laptop | yes. I am not 100 percent sure of the process, but it does entail several steps and watining | 18:22.04 |
Robin_Watts | mvrhel_laptop: So IMHO it won't hurt to get started on that. | 18:22.29 |
mvrhel_laptop | Robin_Watts: ok. I will start that process then. | 18:22.57 |
Robin_Watts | mvrhel_laptop: In the new commit... | 18:24.50 |
| ScrollChanged seems much simpler now. | 18:24.58 |
| Where did the test for whether you are using the display lust or not go? | 18:25.16 |
| I'm just going to give up and type lust all the time. | 18:25.26 |
mvrhel_laptop | Robin_Watts: ha | 18:25.32 |
| Robin_Watts: there is a test, to see if the list is already there during the call to create the list | 18:25.50 |
Robin_Watts | In CreateDisplayList ? | 18:26.06 |
mvrhel_laptop | and if it is we simply return and the thread continuation will occur to render it | 18:26.07 |
| yes | 18:26.09 |
Robin_Watts | http://git.ghostscript.com/?p=user/mvrhel/mupdf.git;a=commitdiff;h=39265766d033d9008f9e5920b22e821ca9e84566 | 18:26.34 |
| In muctx.cpp ? | 18:26.39 |
mvrhel_laptop | no in mudocument.cpp | 18:26.59 |
| hmm | 18:27.12 |
| hold on | 18:27.15 |
| wtf | 18:27.42 |
| h there it is | 18:28.13 |
| in CreateDisplayList | 18:28.19 |
| it is all merged in the + stuff | 18:28.27 |
| + if (mu_object.GetDisplayListPage() == page_num) | 18:28.37 |
| + return S_ISOK; | 18:28.39 |
Robin_Watts | Isn't that "if (the page we have a display list for == this page)" ? | 18:29.07 |
mvrhel_laptop | yes | 18:29.14 |
Robin_Watts | rather than "if (we are using a display list)" ? | 18:29.17 |
mvrhel_laptop | yes | 18:29.27 |
Robin_Watts | oh, this code is always using a display list? | 18:29.27 |
| Sorry, being dim. Gotcha. | 18:29.36 |
| And my next stupid question... | 18:30.06 |
| std::lock_guard<std::mutex> lock(mutex_lock); | 18:30.14 |
| What's that do ? | 18:30.17 |
| does there need to be a matching unlock ? | 18:31.23 |
mvrhel_laptop | Robin_Watts: no, it only has the life of the function | 18:31.36 |
Robin_Watts | so it's to prevent 2 simultaneous calls to CreateDisplayList ? | 18:31.55 |
mvrhel_laptop | yes | 18:32.00 |
| well | 18:32.04 |
| in the same thread | 18:32.16 |
Robin_Watts | Is there potentially a race condition here where 2 things might call this function for different pages at the same time? | 18:33.30 |
mvrhel_laptop | I am wondering though if I need these, since you have all the locks inside mupdf | 18:33.32 |
Robin_Watts | The first one would check to see if we have the right displaylist, and (supposing we do) would return. | 18:33.59 |
mvrhel_laptop | oh. I can see where you are going | 18:34.24 |
| the page could change | 18:34.39 |
Robin_Watts | Then the second one would see that we don't have the right one for it's page, and would bin it, and make a new one? | 18:34.41 |
| right. | 18:34.44 |
mvrhel_laptop | yes | 18:34.45 |
| ok, let me move this inside the lock | 18:34.52 |
Robin_Watts | Even moving the check inside the lock doesn't help. | 18:35.04 |
| cos when we leave this function the displaylist could still be changed before it's used. | 18:35.31 |
mvrhel_laptop | oh yes, I suppose that could happen | 18:36.19 |
Robin_Watts | We'd need to lock something to cover from checking for the existence/correctness of the displaylust to when we finish using it. | 18:36.20 |
| Assuming that this can ever be called twice at the same time - maybe it's protected from that elsewhere? | 18:36.40 |
mvrhel_laptop | let me think about this for a second | 18:37.11 |
| oh shoot, I have to go pick up the kids from school early today | 18:37.22 |
| I will be back in a bit. and think about this... | 18:37.33 |
Robin_Watts | no worries. | 18:37.38 |
mvrhel_laptop | bbs | 18:37.42 |
Robin_Watts | Possibly we ought to have display lists as being refcounted things. | 18:38.13 |
| that way you could take a reference under the lock, and drop it after you render and you'd be protected. | 18:38.43 |
| By having them storable we could potentially cache them nicely too. | 18:39.03 |
| tor8: ^ | 18:39.07 |
| tor8: The //XXX change has broken tiling. | 18:46.07 |
tor8 | Robin_Watts: hmm. | 18:47.02 |
Robin_Watts | Which explains the 204 diffs. | 18:47.40 |
| it doesn't explain the SEGVs seen in the next commit though :( | 18:47.49 |
| Today has not been a good day. | 18:48.02 |
| Tomorrow is not looking good either. | 18:48.10 |
tor8 | damn these pattern transform spaces. my brain is still too mushy from travel to sort them out properly... | 18:51.34 |
| SEGV could happen if begin/end page is not called | 18:52.27 |
Robin_Watts | tor8: ok, commits on robin/master | 18:53.25 |
| First one is the tiling fix. | 18:53.34 |
| Then the next one is the pdf parsing fix that you've seen and didn't like. | 18:53.47 |
| the one after that is a change to hopefully make you like it. | 18:54.03 |
| If you approve we can squash them before commit. | 18:54.12 |
| ok, the SEGV is due to add_span_to_soup being called with no soup bowl. | 18:58.46 |
tor8 | Robin_Watts: that indicates I've forgotten a place where begin_page and/or end_page should be called | 18:59.35 |
Robin_Watts | fz_run_page_contents ? | 19:00.33 |
tor8 | in alternative fix, next_tok is never set or used... | 19:00.35 |
Robin_Watts | tor8: Damn. I meant to remove that. | 19:00.48 |
| I ended up using a goto. Neither approach was pretty. | 19:01.01 |
tor8 | who calls fz_run_page_contents manually? | 19:01.05 |
Robin_Watts | pdfapp_loadpage ? | 19:01.23 |
tor8 | ohh! | 19:01.29 |
| only tested with mudraw | 19:01.37 |
Robin_Watts | We discussed having the dev_null level call begin_page/end_page automatically. | 19:02.09 |
| but maybe we decided that was impossible cos of the need for the mediabox. | 19:02.23 |
tor8 | yeah. it would've been awkward. | 19:02.41 |
| the pdfapp page could bracket the calls to run_display_list with begin/end page | 19:04.15 |
| s/page// | 19:04.26 |
Robin_Watts | just testing that now. | 19:04.58 |
tor8 | fix for SEGV in pdfapp on tor/master | 19:09.47 |
Robin_Watts | and on robin/master :) | 19:10.03 |
| Yours is nicer. I will adopt it. | 19:10.40 |
tor8 | the tiling breakage fix is okay | 19:11.30 |
Robin_Watts | ok, the tiling fix, and your SEGV fix both pushed. | 19:13.13 |
mvrhel_laptop | Robin_Watts: I am back, though I need to eat lunch. Thinking about this a bit, I really need a lock and release around any access to the display list member variable. It is essentially, like a display list cache with one entry. | 19:44.54 |
ray_laptop | mvrhel_laptop: please check my repo on casper and tell me if it seems OK to you. This does fix the customer's issue for a color page after the first color page. | 19:45.04 |
mvrhel_laptop | in its current form | 19:45.05 |
Robin_Watts | right. | 19:45.17 |
mvrhel_laptop | ray_laptop: ok | 19:45.20 |
Robin_Watts | you need to lock, lookup, use, unlock, right? | 19:45.36 |
| or even lock, lookup, if not there {free, regenerate}, use, unlock | 19:46.09 |
mvrhel_laptop | Robin_Watts: yes. so let me fix that up, which may take me a day or so and merge that into this commit too :) | 19:46.16 |
Robin_Watts | It seems to me that we could maybe make this easier for you by having display lusts being reference counted things. | 19:46.47 |
ray_laptop | Robin_Watts: is it OK to have the lock set for a long time ? | 19:46.54 |
mvrhel_laptop | Robin_Watts: yes. that would be a good thing | 19:47.16 |
Robin_Watts | ray_laptop: The lock in question here is one specific to the winrt app, not a core mupdf thing. | 19:47.22 |
| mvrhel_laptop: I will look into that tomorrow. It may be relatively trivial. | 19:47.44 |
mvrhel_laptop | Robin_Watts: ok great. thank you | 19:47.51 |
ray_laptop | Robin_Watts: ok, thanks. I just tend to avoid locks that are long lived | 19:47.53 |
mvrhel_laptop | the lock would only be on while we are creating the list or rendering with it | 19:48.14 |
Robin_Watts | ray_laptop: Indeed. Mupdfs locking is careful to try to lock for the minimum time possible. | 19:48.18 |
| By having reference counting we can avoid holding the lock for too long. | 19:48.36 |
| I'd like tor8's opinion on having display lists being reference counted. | 19:48.57 |
ray_laptop | mvrhel_laptop: I'm going to lunch as well. Call me if you find something. Otherwise I'll email the customer the changes this afternoon (their AM) | 19:49.00 |
Robin_Watts | I can't think of a downside offhand. | 19:49.11 |
ray_laptop | like ref counting. | 19:49.17 |
mvrhel_laptop | ray_laptop: ok. let me eat first. then I will review. low blood sugar here.. | 19:49.20 |
ray_laptop | mvrhel_laptop: np. no rush. They probably don't start work for another 5 hours or so | 19:49.53 |
| bbiaw | 19:50.16 |
tor8 | Robin_Watts: refcounted display lists, yes. | 20:19.21 |
mvrhel_laptop | ray_laptop: changes look good to me. | 20:42.51 |
ray_laptop | mvrhel_laptop: thanks for having a look. I'm running a regression (mostly to make sure it compiles) | 21:01.10 |
| which is apparently did :-) | 21:01.25 |
mvrhel_laptop | ray_laptop: great! | 21:01.30 |
ray_laptop | We probably should add the pnmcmyk with -dGrayDetection=true to the regression run (weekly?) | 21:02.22 |
mvrhel_laptop | ray_laptop: perhaps. there are a pile of color options that I need to talk with marcosw about at some point | 21:11.54 |
| Forward 1 day (to 2013/06/13)>>> | |