| | <<<Back 1 day (to 2012/04/25) | 2012/04/26 |
| mvrhel_laptop | Robin_Watts: for the logs. Thanks for the tip on the pattern clist issue. I believe I fixed it | 05:22.42 |
| Robin_Watts | Morning tor8 | 09:09.30 |
| | Did you see my burblings last night about 2 new commits in my casper repo ? | 09:09.47 |
| tor8 | morning Robin. yes, I saw :) | 09:10.51 |
| | so, pdfposter is the equivalent of psnup? | 09:11.21 |
| Robin_Watts | yeah. | 09:12.27 |
| tor8 | I think it'd be swell to have an equivalent of psbook as well, for making signatures to print books and booklets. | 09:12.28 |
| Robin_Watts | I wanted mupdfposter so I could print things like http://yourlogicalfallacyis.com/poster | 09:13.08 |
| | tor8: No, wait... psnup is the opposite I think. | 09:13.42 |
| tor8 | Robin_Watts: oh, so it splits one page into many, not put many pages on one paper? | 09:13.44 |
| | yeah, psnup is the opposite :) | 09:13.52 |
| | but I agree, all those tools are things we could do | 09:14.06 |
| | pdfnup is more complicated because of the content streams. one way would be to turn each page into an XObject form and then synthesize a new page content streams that place those xobjects on the big page | 09:15.14 |
| | you forgot to add mupdfposter.c in your commit though | 09:15.45 |
| Robin_Watts | pushed. | 09:16.43 |
| | pdfnup: yeah; I was thinking we'd do it by making wrapper content stream fragments. | 09:17.12 |
| | I don't follow what psbook does from the man page though. | 09:17.32 |
| tor8 | it puts two pages to a page (so two portrait pages end up side-by-side in landscape) and rearranges the page ordering | 09:19.11 |
| | so that if you print them out in duplex, you can take a stack of N sheets, fold them in the middle, and you have a "signature" for binding into a book | 09:19.39 |
| Robin_Watts | tor8: Ah, what I've just read suggests that psbook rearranges page order, and psnup does the "2 onto 1" thing. | 09:20.05 |
| tor8 | yeah, could be that you need both tools | 09:20.24 |
| kens | tor8 don't forget that a signature can be more complex than 2-up | 09:21.05 |
| | And pahes need not be oriented in teh 4 basic orientations | 09:21.18 |
| | paper folding can result in small angles being required | 09:21.30 |
| tor8 | Robin_Watts: check out mupdf 0.3 and you can see the old pdf editing and object updating functions | 09:21.32 |
| | might be worth just bringing those back from the dead | 09:21.38 |
| | there's all the funky stuff to add and update objects in the xref and save them back out | 09:21.59 |
| | also for updating stream contents | 09:22.12 |
| | kens: yeah, but I was trying to get the basic idea across :) | 09:22.43 |
| Robin_Watts | I had to add a pdf_new_ref thing that makes a new xref entry for a given object. | 09:22.53 |
| tor8 | Robin_Watts: http://git.ghostscript.com/?p=mupdf.git;a=blob;f=mupdf/pdf_xref.c;h=a2ad01cae22e00a2cb9fabb08eba5d6fb996f6ba;hb=5d500fad85344dcad7e6c0cf0463f5d58f6acea3#l193 and down | 09:23.47 |
| | http://git.ghostscript.com/?p=mupdf.git;a=blob;f=mupdf/pdf_save.c;h=2c1e84aed75c75ed29b1f0e7cf395845cbed431d;hb=5d500fad85344dcad7e6c0cf0463f5d58f6acea3 the whole file | 09:24.16 |
| | http://git.ghostscript.com/?p=mupdf.git;a=blob;f=mupdf/pdf_doctor.c;h=12ee46a1e96c413f8469eb32a5dbb728bdba874e;hb=5d500fad85344dcad7e6c0cf0463f5d58f6acea3 and the garbage collection stuff (probably buggier and more broken than the stuff in pdfclean, but it's the same code) | 09:24.47 |
| | the "transplant" function was used in a tool to merge multiple pdf files into one | 09:25.14 |
| | pdfcat basically | 09:25.23 |
| | Robin_Watts: right, so each of the poster sub-pages just has a modified mediabox? | 09:30.11 |
| Robin_Watts | tor8: Yes. | 09:51.08 |
| vipul_maha | hi, i am searching for a ps creator in linux in which i can pass text and do formatting like bold, italics, alignment etc | 13:52.56 |
| | can anyone suggest one??? | 13:53.03 |
| kens | Any decent word processing application | 13:53.19 |
| | Or TeX ? | 13:53.25 |
| vipul_maha | I have tried Latex but it has its own limitations......like irregular spacing, special characters escape sequences, no auto wrap around etc | 13:55.27 |
| | i used groff also | 13:55.33 |
| | but i want features like bold, italics, underline, various font types, sizes, alignment (left, right, center) page orientation, page margin, auto vertical spacing according to font size , auto wrap around | 13:56.58 |
| | I can't find one application which do all this.......both groff and latex has one or the other problem | 13:57.35 |
| Robin_Watts | sounds like you want html + css -> ps | 14:02.41 |
| | If only someone had written something to let you view html + css and print to a printer.... | 14:03.07 |
| | mvrhel_laptop: Hey | 14:03.14 |
| mvrhel_laptop | hi Robin_Watts | 14:03.20 |
| Robin_Watts | So you solved one of the problems? | 14:03.25 |
| | I couldn't immediately spot the fix in the diff. | 14:03.43 |
| mvrhel_laptop | yes. So everything is fixed except for one issue I believe | 14:03.51 |
| | tests_private/comparefiles/Bug692517.pdf.psdcmyk.72.0 peeves Seg_Fault | 14:03.55 |
| | the other segvs also happen in the trunk so I am not going to worry about those | 14:04.10 |
| | this one is interesting though | 14:04.19 |
| | I can't even open the file with Acrobat | 14:04.25 |
| | Robin_Watts: so when I run this file on windows, during a clist play back of some bit maps from copy_planes there is a bad op which causes things to go south | 14:05.11 |
| Robin_Watts | ok... | 14:06.03 |
| | Only on windows? | 14:06.17 |
| | I mean, does a valgrind run show anything, or memento ? | 14:06.32 |
| mvrhel_laptop | It is a bit map with 14 planes and it gets a bad opcode for a compression value. Oh I think the same thing will happen in linux. From that point, the file will segv in windows too | 14:06.56 |
| Robin_Watts | If you give me an up to date patch, I'll have a look too (presumably you are about to go into the meeting again) | 14:07.11 |
| mvrhel_laptop | its is just that things start to cascade in the wrong direction at that point | 14:07.19 |
| | yeah. I am going to go in about an hout. | 14:07.32 |
| | hour | 14:07.35 |
| | let me send you the latest patch | 14:08.20 |
| | so you will want to do some snooping around cmd_op_copy_mono_planes | 14:12.43 |
| | and see where the command is created and reconstructed | 14:13.00 |
| Robin_Watts | Just updating Memento so you can do Memento_breakOnFree(address) or Memento_breakOnRealloc(address). | 14:14.19 |
| mvrhel_laptop | the bad_op occurs at 1056 in gxclrast.c | 14:14.31 |
| | where it reads back garbage for the compression type | 14:14.52 |
| | to solve this one, it is going to be a matter of looking at the memory buffer created by clist writing and making sure it all makes sense, and then catching where things get out-of-sync during playback | 14:15.55 |
| | the big issue for me is catching the right bitmap | 14:16.07 |
| | since I can't open the file with Acrobat, I can't cut it down | 14:16.18 |
| | does the file open for you? | 14:16.23 |
| | Robin_Watts ^^ | 14:16.33 |
| Robin_Watts | Oh, I see. I may be able to cut it down. | 14:16.35 |
| | I don't use Acrobat to do such things. | 14:16.41 |
| | Let me see. | 14:16.45 |
| | I cannot open it with Acrobat. | 14:18.06 |
| mvrhel_laptop | ok | 14:18.10 |
| | that makes me almost not want to worry about this file | 14:18.24 |
| Robin_Watts | Ah, it's ps, not PDF :) | 14:18.25 |
| mvrhel_laptop | huh | 14:18.31 |
| | oh | 14:18.32 |
| | it was misnamed | 14:18.38 |
| | that explains now why the number of planes is 14 in everything | 14:18.52 |
| Robin_Watts | Rename it to .ps and acrobat reads it fine. | 14:19.34 |
| mvrhel_laptop | yes. now the question is, how is my postscript editing. | 14:19.59 |
| | I bet the PDF version runs just fine | 14:20.08 |
| | at 7 Meg, I am not too interested in going into the PS file and cutting it down | 14:21.36 |
| Robin_Watts | I will try that. | 14:21.46 |
| mvrhel_laptop | a brave man steps forward | 14:21.56 |
| Robin_Watts | s/brave/foolish/ | 14:22.05 |
| | I may not get very far :) | 14:22.13 |
| | ok, building with your patch now. | 14:23.40 |
| | oh. It's a non-banded job. | 14:28.23 |
| kens | thanks for the memento change Robin | 14:28.43 |
| Robin_Watts | and doesn't crash for me. | 14:28.51 |
| | kens: Oh, you're welcome. | 14:28.59 |
| | mvrhel_laptop: What's your command line please? | 14:29.19 |
| mvrhel_laptop | -sDEVICE=psdcmyk -r72 -o ./myoutputs/test3.psd ./myinputs/sep/Bug692517.ps | 14:29.38 |
| | It looks like there is only 1 cmd_op_copy_mono_planes command that is pickled into the clist, so this may not be too hard to figure out | 14:30.10 |
| Robin_Watts | rebuilds. | 14:30.41 |
| mvrhel_laptop | basically a break point in clist_copy_planes and near cmd_op_copy_mono_planes in gxclrast.c is what we want | 14:32.23 |
| sebras | Robin_Watts paulgardiner: your fixes doesn't solve the entire android issue for me... | 14:33.19 |
| paulgardiner | sebras: Is that the crash? | 14:33.47 |
| Robin_Watts | mvrhel_laptop: Reproducing the crash is the first step. | 14:33.51 |
| sebras | paulgardiner: yes. | 14:33.56 |
| Robin_Watts | There are other places where the async task stuff is used. I just patched one. | 14:34.23 |
| | With Pauls (neater) fix it may be easier to patch the rest too. | 14:34.52 |
| sebras | paulgardiner: http://pastebin.com/nP4dCgyC | 14:35.16 |
| | paulgardiner: so essentially we run out of memory, the app crashes and then later libmupdf is still rendering... | 14:35.34 |
| Robin_Watts | mvrhel_laptop: Ah. Interesting one... with your command line I get the crash. | 14:35.36 |
| | Add -dMaxBitmap=10000000 and you don't. | 14:35.46 |
| | That means you're seeing the crash in the banded case, not the unbanded case. | 14:36.01 |
| | Which means you've found a crash that isn't the one that the cluster is reporting. | 14:36.22 |
| mvrhel_laptop | oh | 14:37.43 |
| | I see | 14:37.56 |
| paulgardiner | sebras: Any signs as to why we are running out of memory? | 14:39.13 |
| mvrhel_laptop | so, I am actually not seeing the crash at all | 14:39.37 |
| Robin_Watts | mvrhel_laptop: I don't see a crash in the banded case on windows. I do in the unbanded case. | 14:40.09 |
| sebras | paulgardiner: the testcase is the same as I reported before 1.0 -- repeatedly clicking to a new page. | 14:40.13 |
| mvrhel_laptop | oh | 14:40.16 |
| Robin_Watts | I suspect a random memory overwrite thing, which is showing up differently on windows and 64bit linux. | 14:40.35 |
| sebras | paulgardiner: I guess there are just too many page renders in progress. | 14:40.44 |
| Robin_Watts | The clist may just be the poor unfortunate that gets in the way. | 14:40.54 |
| mvrhel_laptop | yes | 14:40.59 |
| | well, it is interesting. the command starts to parse correctly | 14:41.15 |
| | and then when it goes to get the actual bit map data, things are not correct | 14:41.35 |
| paulgardiner | sebras: Yeah, I think you are right. I create a PatchInfo object, that refers to a bitmap, and the system wont be able to gc it until the task completes. | 14:45.21 |
| sebras | paulgardiner: so is it possible to try to allocate this and handle the error condition in any other way than OOMing? | 14:46.18 |
| Robin_Watts | The correct solution is just not to queue so many jobs. | 14:46.44 |
| | 'just' | 14:46.46 |
| sebras | Robin_Watts paulgardiner: is this the neater suggestion you mentioned earlier? | 14:47.43 |
| paulgardiner | Robin_Watts: Yes. I guess we should perform them in the foreground beyond a certain count | 14:47.50 |
| Robin_Watts | sebras: No. Paul did a neater version of my fix - it's in his repo. | 14:48.38 |
| sebras | paulgardiner: and the threshold likely ought to be relatively low. maybe 5..? | 14:48.39 |
| Robin_Watts | paulgardiner: I disagree. | 14:48.51 |
| | Whenever we want a job done, we currently bung it into the background. | 14:49.32 |
| | if another job comes in, we bung that one into the background too. | 14:49.47 |
| | When another one comes in, we may now know we don't need one of the ones that's in the background already. | 14:50.18 |
| paulgardiner | But we have no way to cancel it quickly | 14:50.39 |
| Robin_Watts | In that case we should either cancel that one (or if it's not started yet) repurpose it to do the new job. | 14:50.55 |
| paulgardiner | I believe I already cancel jobs as soon as they are not needed: i.e., when the page is no longer within the three to be cached,. | 14:51.46 |
| | But cancelling is an asynchronous action. | 14:52.09 |
| | Plus the way things are at the moment, the task actually runs to completlion before truly cancelling | 14:52.46 |
| | Actually, I'm not completely sure I cancel in the condition I stated above. It might be an idea if I check,. | 14:53.40 |
| sebras | paulgardiner: this OOM also makes me wonder -- is it possible to end up crashing mupdf by looking at a pdf zoomed in enought to request a bitmap that is larger than the heap given to each app by android? | 14:56.06 |
| paulgardiner | sebras: No. That case is dealt with. We have two bitmaps, both screen sized. One contains an image of the whole page, and hence has to be scaled up (and is thus blurry); the other contains an image of just the part on screen. | 14:57.58 |
| | sebras: we never allocate a bitmap larger than the screen | 14:58.18 |
| sebras | paulgardiner: right, though that doesn't guarantee that the allocation is within the allotted heap budget. ;) | 14:59.14 |
| Robin_Watts | mvrhel_laptop: I have the file down to 1Meg now. | 14:59.27 |
| mvrhel_laptop | thats an improvement | 14:59.39 |
| paulgardiner | sebras: No, but we should be ok if we can stop wasting bitmaps on no-longer-needed bms. | 15:00.52 |
| Robin_Watts | I'm not convinced how many of my wifes piano pupils actually have a future as pianists. I think they have more potential as dog trainers: "Drop. Down. Paw. Good girl!" | 15:03.23 |
| paulgardiner | sebras, Robin_Watts: Actually it looks like I'm not cancelling: ReaderView releases all references to the page when it goes out of the 3-page cache, but there is no explicit cancel. I'll see if I can sort that out. | 15:05.13 |
| mvrhel_laptop | Robin_Watts: so I believe I have found the clist issue | 15:12.40 |
| | which is, likely unrelated to the crash | 15:13.05 |
| | clearly something else is wrong as now this thing is taking forever to run | 15:13.54 |
| | oh | 15:14.12 |
| | wait | 15:14.15 |
| | Robin_Watts: need to go. | 15:15.46 |
| Robin_Watts | cu. | 15:15.52 |
| mvrhel_laptop | I will talk with you later | 15:15.54 |
| Robin_Watts | (sorry, on phone) | 15:15.57 |
| | paulgardiner: At some point I should probably talk you through using the cluster. | 15:32.25 |
| paulgardiner | Robin_Watts: Yeah, ta. That would be good. | 15:32.45 |
| Robin_Watts | Are you around tomorrow? | 15:33.02 |
| paulgardiner | Robin_Watts: yes | 15:35.08 |
| Robin_Watts | ok, let's do it then. | 15:35.17 |
| paulgardiner | sebras: just pushed another commit to casper. Not sure whether it will help much because we don't yet have the hooks in place to cancel rendering quickly, but at least it does now cancel the AsyncTask, so worth a try, | 15:39.15 |
| sebras | paulgardiner: I'm unable to build the android app at the moment so if you want immediate response then I need an .apk. | 15:40.10 |
| paulgardiner | sebras: no need for quick response, but I can certainly send you the .apk if it helps you at all. | 15:41.38 |
| sebras | paulgardiner: ok. then let's wait until I'm at home. | 15:43.07 |
| paulgardiner | sebras: Sure. Thanks. | 15:43.21 |
| Robin_Watts | It looks to me like mvrhels crash IS releated to the clist stuff. So if he's fixed that, there is no point in me pursuing it. | 15:44.28 |
| paulgardiner | sebras, Robin_Watts: I think making the cancelling of the AsyncTask truly interrupt the render will cure these problems, and is something we should do at some stage. | 15:45.37 |
| Robin_Watts | paulgardiner: Is that possible ? | 15:46.08 |
| paulgardiner | Robin_Watts: Oh possibly not then. I thought it was you who told me it was. :-) | 15:46.39 |
| Robin_Watts | tor said that in ios he cancels tasks. | 15:47.07 |
| paulgardiner | Robin_Watts: Isn't there a flag or something you can set to shortcut a render. | 15:47.08 |
| | Robin_Watts: I didn't mean interrupt in the "stop the proces" sense. | 15:47.23 |
| Robin_Watts | paulgardiner: Oh, yes, we can shortcut a render. | 15:47.24 |
| | I was thinking "cancel a queued async task before it starts" | 15:47.41 |
| paulgardiner | Robin_Watts: Ah right. | 15:47.53 |
| Robin_Watts | Anything that involves not really removing async tasks from the queue (or repurposing them) is a partial fix at best, IMAO. | 15:54.42 |
| | ooh, marcosw, didn't see you arrive. | 15:55.08 |
| | I've pretty much got rid of all the mupdf cluster indeterminisms, except for occasionally when jobs run on feet. | 15:55.47 |
| marcosw | Robin_Watts: morning. I'm lecturing in about 10 minutes, so am not really here :-) | 15:55.53 |
| Robin_Watts | Any chance I can get a login for feet at some point please? | 15:56.12 |
| | no hurry. | 15:56.49 |
| kens | Time for me to go, bye all | 15:57.47 |
| Robin_Watts | hehe. Cutting down this file, I'm going further and further through the structure, boggling at the levels of nesting (patterns in smasks in xobjects in patterns in smasks in xobjects). How can anything sane generate this? Oh... it's a cairo file... | 16:35.44 |
| ray_laptop | Wow. By fixing pdf_draw.ps cust 532's file went from over 7,000 garbage collections for a single page to only 9. | 17:16.17 |
| | should make Len happy | 17:16.28 |
| Robin_Watts | ray_laptop: Excellent! | 17:16.51 |
| ray_laptop | we were doing the 'settransfer' even when the currenttransfer was already the same as what we were going to 'set'. Running their transfer function proc loaded a 256 element array which then had to be GC'ed | 17:18.12 |
| Robin_Watts | I'm confused. I've shrunk this file as much as I can. | 17:21.47 |
| | http://ghostscript.com/~robin/smaller.pdf | 17:21.58 |
| ray_laptop | we never saw it because the default transfer function we use is either the identity procedure or a calculated function. Should speed up the cases where we had the calculated function (dpi > 150) | 17:22.04 |
| Robin_Watts | One of the patterns has a graphics state defined, which it never uses. | 17:22.45 |
| | If I remove the reference to that graphics state... it stops crashing. | 17:22.58 |
| | I guess the pdf interpreter must walk the references to figure out if a pattern is transparent or not, and maybe the call route through the code is different? | 17:24.22 |
| ray_laptop | Robin_Watts: yes, we do have to examine patterns to see if they use transparency | 17:24.49 |
| Robin_Watts | This is a clist pattern in a clist. | 17:25.24 |
| | Well it's an SMask containing a pattern which contains a pattern. | 17:26.29 |
| ray_laptop | Robin_Watts: in pdf_main.ps the /patternusestransparency procedure checks the ExtGStates using 'extgstateusestransparency' | 17:26.43 |
| | Robin_Watts: is it a SEGV crash, or just the PDF interpreter choking ? | 17:27.58 |
| Robin_Watts | SEGV. | 17:28.04 |
| ray_laptop | interesting. | 17:28.17 |
| Robin_Watts | The clist playback is causing a call to be made to gx_dc_pattern_load with pinst == NULL. | 17:28.27 |
| | (device color -> ccolor . pinst) | 17:28.47 |
| | (device color -> ccolor . pattern even) | 17:29.03 |
| ray_laptop | Robin_Watts: so the pattern didn't get written to that band ? | 17:30.45 |
| Robin_Watts | Could be. I'm fumbling in the dark a bit. | 17:31.14 |
| ray_laptop | Robin_Watts: does it run if you put everything in a single band ? (-dBandHeight=page_height) | 17:31.30 |
| Robin_Watts | I was basically trying to reduce the problem enough so that hopefully someone else could nudge it over the line. | 17:31.38 |
| | gswin32c.exe -sDEVICE=psdcmyk -o out.psd -r300 -dMaxBitmap=10000 smaller.pdf | 17:33.01 |
| | That dies. | 17:33.03 |
| | If I try and up the -dBandHeight too much, it fails in a gs_malloc | 17:33.29 |
| ray_laptop | Robin_Watts: OK. Only with psdcmyk ? | 17:33.38 |
| Robin_Watts | So far. (it fails with psdcmyk both with and without mvrhels changes) | 17:33.57 |
| ray_laptop | Robin_Watts: yes, it tries to allocate a band that big. If it is a 'letter' page at 300 dpi then 3301 is enough | 17:34.35 |
| Robin_Watts | Dies with tiffsep too, but not ppmraw or bitcmyk | 17:34.57 |
| ray_laptop | Robin_Watts: you can use -Z: in a debug build to see nbands | 17:35.11 |
| Robin_Watts | with dBandHeight=10000 it gets past the malloc and then SEGVs the same way. | 17:35.34 |
| ray_laptop | Robin_Watts: not too surprising. all other devices ignore DeviceN colors (except one mode of the Windows Display device) | 17:35.50 |
| | other devices use the tint transform to map spot colors to another colorspace | 17:36.24 |
| Robin_Watts | So, this is smelling like a clist issue to me. | 17:36.33 |
| | no spots in the file though. | 17:36.43 |
| ray_laptop | smells like a color issue to me. | 17:36.51 |
| | really? no DeviceN colorspace stuff ??? | 17:37.10 |
| Robin_Watts | Let me double check. | 17:38.32 |
| ray_laptop | the pdf14 device does a lot of screwy stuff changing colorspaces as it goes along. Getting the colorspace setting in the clist is tricky -- there were several bugs along the way where it wasn't being properly tracked | 17:38.47 |
| Robin_Watts | nope, DeviceRGB all the way through. | 17:39.00 |
| ray_laptop | Robin_Watts: which file | 17:39.01 |
| Robin_Watts | http://ghostscript.com/~robin/smaller.pdf | 17:39.13 |
| ray_laptop | is waiting for a cluster run, so can take a look | 17:39.19 |
| Robin_Watts | That's my vastly reduced version. | 17:39.26 |
| | tests_private/comparefiles/Bug692217.pdf is the original. | 17:39.52 |
| ray_laptop | Robin_Watts: how come it looks like a blank page ? | 17:39.56 |
| Robin_Watts | ray_laptop: Vastly reduced, see :) | 17:40.07 |
| ray_laptop | in the words of Sgt. Schultz "I see nothink" | 17:40.41 |
| Robin_Watts | But does it SEGV for you using the same command line as me ? | 17:41.02 |
| ray_laptop | yes | 17:43.31 |
| Robin_Watts | I can give you a version that's not blank if you prefer | 17:43.46 |
| ray_laptop | sure is reduced. the cfile (with -dBandHeight=3550) is only 4730 bytes | 17:44.14 |
| ray_laptop | tries -ZL | 17:44.25 |
| | it dies right after a 'put_params' after 'Pattern clist playback' begins. | 17:46.40 |
| | Robin_Watts: thanks for reducing the test case so much | 17:47.16 |
| Robin_Watts | ray_laptop: I was hoping that I could shrink it to be small enough to fit in my head, but... no. | 17:48.24 |
| | It's a clist pattern in a clist, so I'm trebly confused. | 17:49.21 |
| | hi mvrhel_laptop. | 17:50.40 |
| mvrhel_laptop | hi Robin_Watts: | 17:50.52 |
| | so I had a few questions for you | 17:50.57 |
| Robin_Watts | I gave up looking at the problem cos you said you'd found it. | 17:51.01 |
| mvrhel_laptop | well, I have found the issue but I am rather confused | 17:51.15 |
| ray_laptop | Robin_Watts: yes, the memory based clist get serialized into the disk clist | 17:51.16 |
| Robin_Watts | I've reduced one of the ones that crashes on the trunk too, and ray is looking at that. | 17:51.35 |
| ray_laptop | the pattern clist is only 144 bytes | 17:52.05 |
| Robin_Watts | mvrhel_laptop: Well, confusion shared is confusion doubled, so fire away... :) | 17:52.14 |
| mvrhel_laptop | Robin_Watts: ok. So in gxp1fill.c at line 248 is the call to copy_planes | 17:52.35 |
| | this makes its way through to the clist | 17:52.48 |
| | where it proceeds to treat the width as if it were a bit raster in writing out the data | 17:53.11 |
| | to the clist | 17:53.13 |
| | later during playback, we grab the width as if it were bytes | 17:53.32 |
| Robin_Watts | Hold on. I need to reapply your patch. | 17:53.45 |
| | OK, that makes more sense :) | 17:54.36 |
| mvrhel_laptop | ok. so put a break point at 252 and check all the width height etc stuff | 17:55.22 |
| Robin_Watts | 252 where ? | 17:55.36 |
| mvrhel_laptop | oops | 17:55.49 |
| | in gxp1fill.c | 17:55.54 |
| Robin_Watts | sorry, gxp1fill.c | 17:55.55 |
| mvrhel_laptop | then you can put one at line 903 in gxclrect.c | 17:56.25 |
| Robin_Watts | Gah. Have to rebuild. | 17:57.00 |
| mvrhel_laptop | one follows the other but this is the offending write into the clist. It is not clear to me who is correct, the read or the write, but I suspect something is wrong during the writing phase, since the tile is bytes of data with width 7 not bits with width 7 | 17:57.33 |
| | It is likely that the call at line 252 is the issue | 17:57.59 |
| | since that is the new code | 17:58.06 |
| | oh. | 17:58.21 |
| | I think I know | 17:58.23 |
| Robin_Watts | OK, so I'm at 252 too. | 18:00.01 |
| mvrhel_laptop | ok. so is width 7 and height 1? | 18:00.26 |
| Robin_Watts | Yes. | 18:00.51 |
| mvrhel_laptop | and the plane height is going to be 635 | 18:00.53 |
| | so this data is bytes | 18:01.03 |
| Robin_Watts | I'm just trying to remember how copy_planes works. | 18:01.55 |
| mvrhel_laptop | me too | 18:02.01 |
| | You added it :) | 18:02.07 |
| | I looked at the planar memory code | 18:02.24 |
| Robin_Watts | The first planes data is at data, offset by sourcex. | 18:02.30 |
| | The second planes data is at data + plane_height * raster offset by sourcex. etc | 18:02.52 |
| mvrhel_laptop | right | 18:02.59 |
| | raster for this tile is 664 | 18:03.11 |
| | which is also its width | 18:03.21 |
| Robin_Watts | I'm not sure that the last arg shouldn't be bits->size.y | 18:03.56 |
| | IIRC bits->rep_height can be an integer divisor of bits ->size.y ? | 18:04.22 |
| mvrhel_laptop | hmm I would have expected it to be the tile height | 18:04.39 |
| | in any event, in this case they are the same value | 18:04.47 |
| Robin_Watts | mvrhel_laptop: size.y is the height of the memory block. | 18:05.18 |
| | The tile can be repeated several times within the memory block to allow for speedy copying out. | 18:05.45 |
| mvrhel_laptop | ah | 18:05.57 |
| Robin_Watts | hence rep_height being an integer divisor of the real height. | 18:06.05 |
| mvrhel_laptop | ok. that makes sense | 18:06.13 |
| Robin_Watts | I think. (All this is subject to the usual lengthy disclaimers about my crap memory/understanding) | 18:06.28 |
| ray_laptop | strange cluster action -- it looks like it aborted my bmpcmp and then restarted it | 18:06.32 |
| Robin_Watts | but as you say, in this case it shouldn't matter. | 18:06.36 |
| mvrhel_laptop | right | 18:06.41 |
| | if you step in to clist_copy_planes | 18:06.55 |
| | Robin_Watts: you will see where there seems to be some confusion if the width means bits | 18:07.17 |
| | when it does cmd_put_bits | 18:07.30 |
| | around line 909 in gxclrect.c | 18:07.46 |
| Robin_Watts | yes, indeed. | 18:08.20 |
| mvrhel_laptop | and I get confused, since it may mean bits if we are dealing with the plank device | 18:08.24 |
| | so, my thought was that the issue is back where we are doing the first copy_planes call | 18:09.04 |
| Robin_Watts | This smells to me like this code is expecting to be copying mono planes. | 18:09.10 |
| mvrhel_laptop | yes | 18:09.13 |
| | exactly | 18:09.16 |
| Robin_Watts | and line 897 looks like that's the case. | 18:09.28 |
| mvrhel_laptop | yes | 18:09.40 |
| Robin_Watts | So, does the clist only know about mono planes ? | 18:10.15 |
| mvrhel_laptop | looking over the commands | 18:11.57 |
| | it looks that way | 18:12.35 |
| Robin_Watts | line 881 does (data_x>>3) so it's clearly doing mono. | 18:12.53 |
| | OK, so how to fix this. | 18:13.00 |
| | Is there any way we can know in clist_copy_planes that we should be copying non-mono data? | 18:13.45 |
| mvrhel_laptop | I was just going to ask the same question :) | 18:13.58 |
| | I suspect color info in dev will tell us | 18:14.27 |
| Robin_Watts | dev->colorinfo.depth /dev->colorinfo.num_components ? Seems a bit icky. | 18:15.42 |
| mvrhel_laptop | yes. | 18:15.57 |
| Robin_Watts | presumably we'll have that same info on the reading side too. | 18:16.26 |
| mvrhel_laptop | yes | 18:16.37 |
| | there is already this operation in the clist reading | 18:16.44 |
| Robin_Watts | So we can repurpose the code to mean copy_planes rather than copy_mono_planes. | 18:16.52 |
| | That calculation is already used in the clist reading ? | 18:17.03 |
| mvrhel_laptop | yes | 18:17.05 |
| | and it ends up reading out bytes | 18:17.10 |
| | hold on | 18:17.14 |
| | let me find the line number | 18:17.17 |
| Robin_Watts | ah, well, let's follow the well trodden path to the scene of the accident then :) | 18:17.29 |
| mvrhel_laptop | line 978 in gxclrast.c | 18:18.09 |
| | question is, did I add that... | 18:18.47 |
| | checking blame | 18:19.12 |
| | Robin_Watts: :) | 18:19.35 |
| Robin_Watts | It's not a hack. It's a carefully crafted cunning software engineered solution. | 18:20.02 |
| ray_laptop | looks like somebody tripped over a power plug in miles' office -- both miles and kilometers went DOWN | 18:20.04 |
| mvrhel_laptop | so, if we want that to work on the reader side, we need to fake things out during the call in gxp1fill.c | 18:20.39 |
| Robin_Watts | We need to patch both writer and reader. | 18:20.57 |
| mvrhel_laptop | or that will not work | 18:20.57 |
| | we need to fix the clist writer | 18:21.11 |
| | to write out the proper amount of data | 18:21.21 |
| | or, do we want to add in a real copy_planes command into the clist | 18:21.54 |
| Robin_Watts | Hmm. Does cmd_put_bits only know about 1bpp images ? | 18:22.08 |
| mvrhel_laptop | apparently | 18:22.14 |
| | and then it makes a decision on each plane if it should compress or not | 18:22.45 |
| | and adds a compression byte for every byte of data during the writing phase | 18:23.02 |
| | so that you know that the next byte is not compressed :) | 18:23.16 |
| | hehe | 18:23.23 |
| | at least in this case, when you have 14 planes and a width of 7 | 18:23.38 |
| Robin_Watts | Well, easiest route is probably to still call cmd_put_bits and disable compression. | 18:23.44 |
| | yeah. | 18:23.46 |
| mvrhel_laptop | Robin_Watts: oh I added copy_planes support for the clist so apparently this is my mess | 18:26.46 |
| | If I were to redo this I would have added a new command op | 18:27.18 |
| Robin_Watts | saved by my cowardice :) | 18:27.26 |
| mvrhel_laptop | I overloaded the copy_mono command | 18:27.44 |
| | if plane_height == 0 it ends up doing copy_mono | 18:28.04 |
| Robin_Watts | yeah. | 18:28.16 |
| mvrhel_laptop | else it assumes we are in a copy_planes case | 18:28.18 |
| Robin_Watts | I have got updated writing code, I think. | 18:28.27 |
| mvrhel_laptop | oh. if you feel you can fix it easily, I am fine with that. If not, I would probably add a new command and not do it this way | 18:29.09 |
| | of course adding a new command in the clist takes a few days for me | 18:29.48 |
| | with the certain debugging required | 18:30.01 |
| Robin_Watts | yeah. | 18:30.15 |
| | Let me tidy this up a bit... | 18:30.27 |
| | Can we assume that bits per plane <= 8 ? | 18:32.19 |
| mvrhel_laptop | we can assume that | 18:32.41 |
| Robin_Watts | ok, so where is the reading code for this... | 18:34.40 |
| mvrhel_laptop | oh set a break point at | 18:35.55 |
| Robin_Watts | oh, wow. I think the reading side is easy. | 18:36.01 |
| mvrhel_laptop | line 937 will catch it in gxclrast.c | 18:36.20 |
| Robin_Watts | So.. what's the best way to test this? | 18:36.44 |
| | Just cluster push it from me I guess ? | 18:36.50 |
| mvrhel_laptop | ah ok that would be fine. | 18:37.02 |
| Robin_Watts | Oh, I could always run the file in question first... | 18:37.24 |
| mvrhel_laptop | that would be even better | 18:37.32 |
| | if we get this taken care of, I will be ready to push this | 18:38.22 |
| | after a final sanity check | 18:38.39 |
| Robin_Watts | balls. Still getting a problem. | 18:39.06 |
| mvrhel_laptop | shoot | 18:39.43 |
| | I am going to be getting on my plane in a bit | 18:39.53 |
| Robin_Watts | mvrhel_laptop: I'll bash on this for a bit more. I'll try and mail you whatever I come up with. | 18:40.36 |
| mvrhel_laptop | ok cool. thanks Robin_Watts | 18:40.45 |
| Robin_Watts | actually, you're not going to want to look at this until tomorrow, are you ? | 18:40.53 |
| mvrhel_laptop | I may take a look at it late tonight. | 18:41.13 |
| | but not the rest of the day | 18:41.21 |
| | if you get something working please clusterpush and run a bmpcmp -t 5 | 18:41.34 |
| Robin_Watts | will do. | 18:41.47 |
| | When reading back, in gxclrast.c I altered line 935. | 18:42.49 |
| | And that correctly calculates depth as 8 | 18:43.05 |
| mvrhel_laptop | ok cool | 18:43.13 |
| Robin_Watts | but then on line 960, it goes back to 1. | 18:43.16 |
| | and I don't understand what that code is doing at all. | 18:43.42 |
| mvrhel_laptop | the clist likely keeps a current tile | 18:45.08 |
| | we likely need to set this state properly | 18:46.09 |
| | in the first read | 18:46.14 |
| | so around line 1097 | 18:47.42 |
| | need to set the bit_depth | 18:48.18 |
| | in the state_tile.head | 18:48.26 |
| | I am guessing | 18:48.37 |
| | plane boarding now.... | 18:48.46 |
| | ttyl | 18:48.49 |
| | Forward 1 day (to 2012/04/27)>>> | |
IRC Logs