| | <<<Back 1 day (to 2017/06/20) | 20170621 |
| tor8 | morning Robin_Watts | 09:09.42 |
| Robin_Watts | tor8: I updated lcms and lcms_squashed with a fix from mvrhel this morning. | 09:11.49 |
| tor8 | Robin_Watts: that fix looks good | 09:17.05 |
| Robin_Watts | just running through your list now. | 09:17.36 |
| | I'm up to list-device.c | 09:17.46 |
| tor8 | most should be trivial | 09:17.52 |
| | the const one could have lots of knock-on effects | 09:18.09 |
| Robin_Watts | "Do we need to keep fz_device_gray" | 09:18.32 |
| tor8 | we do a lot of fz_keep_colorspace when setting it up | 09:18.50 |
| Robin_Watts | I assume you mean "Do we need to fz_keep_colorspace ..." ? | 09:18.51 |
| tor8 | yes, sorry. | 09:18.58 |
| Robin_Watts | No, just double checking I wasn't misreading :) | 09:19.09 |
| tor8 | I think Michael did that change when we briefly experimented with replacing the actual device colorspace objects | 09:19.25 |
| Robin_Watts | Previously fz_device_rgb etc were statically setup. | 09:19.30 |
| | ICC ones aren't now, are they? | 09:19.40 |
| tor8 | ah, yes. I see they are set up as our default ICC profile ones. | 09:20.36 |
| Robin_Watts | fz_set_cmm_engine sets them up using fz_new_icc_colorspace, hence they have to be correctly counted. | 09:20.41 |
| tor8 | when setting the cmm engine | 09:20.47 |
| | okay, fair enough. | 09:21.07 |
| Robin_Watts | Let's come back to the const one at the end, cos as you say, that will have knock on effects. | 09:21.31 |
| tor8 | fill_image_mask and begin_mask missing the color_params flag field looks like an accidental omission | 09:23.19 |
| Robin_Watts | yeah, just fixed that. | 09:24.17 |
| | The fz_colorspace_is_lab_icc one... | 09:25.25 |
| | sorry, brb. | 09:25.59 |
| tor8 | pdf_cal_common ought to throw, not return -1. other things can fail that it doesn't detect, and we should cope with that as well as stick to our usual way of dealing with errors IMO | 09:26.30 |
| Robin_Watts | back. | 09:35.46 |
| | can you explain what you mean about fz_colorspace_is_lab_icc ? | 09:36.07 |
| | oh, I get it. | 09:37.39 |
| | hmm. | 09:39.34 |
| | so load_icc_based looks at the alternate color space to see if it's lab. if it is, then it applies a different clamping to it. | 09:40.05 |
| | a) How does that work in the non icc case. | 09:40.24 |
| | b) is it true that the only lab alternate is default_lab ? | 09:40.43 |
| | ok, ignore b), the new formulation is nicer. | 09:41.30 |
| | and let's ignore a) until we find an example. | 09:42.00 |
| tor8 | I *think* a PDF colorspace can be either /Lab directly (or CalLab maybe) or an ICC profile | 09:43.13 |
| | which would be caught by the fz_colorspace_is_lab | 09:43.58 |
| Robin_Watts | yes. I have all these fixes here. | 09:44.12 |
| tor8 | I wouldn't mind if we grouped all the fz_colorspace_is_xxx functions towards the bottom of the file and made them a bit more consistent if possible | 09:44.28 |
| | some test the to_ccs function, som test the clamp function, etc | 09:44.38 |
| Robin_Watts | So, for pdf_cal_common, let's pass in the colorspace to be used in the failure case. | 09:44.50 |
| tor8 | I'd just throw and let a try-clause in pdf_calgray return fz_device_gray | 09:45.29 |
| Robin_Watts | tor8: and in pdf_calrgb. | 09:45.51 |
| tor8 | try { pdf_cal_common } catch { /* ignore error */ return fz_device_gray(); } | 09:45.53 |
| | and in calrgb and calcmyk | 09:45.57 |
| | oh, there is no calcmyk :) | 09:46.08 |
| Robin_Watts | It's not a public API. | 09:46.09 |
| | resorting to an exception to just ignore it seems nasty. | 09:46.30 |
| | try/catch is not costless. | 09:46.34 |
| | especially if we're always ignoring it. | 09:46.44 |
| tor8 | mixing error handling mechanisms is rather nasty too | 09:46.51 |
| | I'd rather see the error message warning | 09:46.57 |
| | and what if we get a pdf syntax error while loading? | 09:47.06 |
| | (a bit theoretical, we swallow all pdf parsing errors in the pdf_obj accessors) | 09:47.34 |
| Robin_Watts | OK. | 09:48.20 |
| tor8 | knowing *why* we ignore it would be helpful, IMO | 09:48.34 |
| | while at that function, it only tests that Yw == 1.0 | 09:48.50 |
| | it doesn't test that Xw aand Zw > 0 | 09:49.03 |
| | and the blackpoint is clamped, doesn't have a > 0 test | 09:49.22 |
| | if gamma < 0 || gamma == 0 ... why not if gamma <= 0? | 09:50.02 |
| | pdf_calrgb only creates a colorspace if Matrix exists | 09:51.32 |
| | the Matrix entry is optional | 09:51.37 |
| | so that needs fixing too | 09:51.41 |
| | I suspect the nesting is just off by one line | 09:51.59 |
| | if (obj && pdf_is_array && pdf_array_len) can be simplified to if (pdf_array_len() == 9) | 09:52.37 |
| | I can fix, if you want | 09:53.04 |
| Robin_Watts | I'm midway through. | 09:54.31 |
| tor8 | okay, nvm then | 09:55.33 |
| Robin_Watts | http://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=bcd62ba5439940faa5b62dd72a2bfcb4183b2725 | 09:57.07 |
| tor8 | pdf_calcommon will buffer overflow when reading the gamma if a CalGray colorspace has an array | 09:57.08 |
| Robin_Watts | If you want to take over and do the rest of your fixes for that function, that may be best. | 09:57.34 |
| tor8 | lgtm so far, I'll tweak that function from that commit on | 09:58.16 |
| | on to the fz_ensure_pixmap_is_additive FIXME? | 09:59.07 |
| Robin_Watts | yeah, fz_convert_pixmap is supposed to take a color_params struct. | 10:01.31 |
| | but none of the places that call this have one. | 10:02.53 |
| | hence I wanted to check with mvrhel that it was safe to use NULL. | 10:03.10 |
| tor8 | use fz_default_color_params? | 10:03.33 |
| | we ought to be safe to use NULL and the callee should pick sane defaults, but best to check with michael | 10:04.05 |
| Robin_Watts | fz_init_color_params says "set this structure to the initial (default) values" | 10:04.40 |
| | fz_default_color_params says "get the 'current' defaults from the context" | 10:04.57 |
| | Urgh... | 10:07.23 |
| | pdf_run_page_contents_with_usage sets the default_colorspaces in the device from the page. | 10:07.46 |
| | pdf_run_annot_with_usage does not | 10:08.06 |
| | so I should copy that logic across, I think. | 10:08.27 |
| | writepixmap is the same FIXME as before. | 10:10.55 |
| | (using NULL, it *will* pick sane defaults, but does that ruin our colour correctness?) | 10:11.23 |
| | tor8: fz_list_set_default_colorspaces... | 10:13.28 |
| tor8 | I would expect writepixmap to write the original color values, uncorrected | 10:13.47 |
| Robin_Watts | we pass &default_cs2, not default_cs2. | 10:13.52 |
| | tor8: yeah, me too, but the other cases I meant. | 10:14.17 |
| | We can't pass &fz_keep_default_colorspaces(ctx, default_cs); | 10:14.44 |
| tor8 | yeah, that looks plain wrong | 10:14.57 |
| | unless...it's copying the pointer from default_cs2 memory | 10:15.29 |
| | yes, it's memcpy:ing from that address | 10:15.47 |
| | memcpy(out_private, private_data, private_data_len) so now I understand the need for default_cs2 to have a stack allocated pointer to copy from | 10:16.17 |
| Robin_Watts | yeah, took me a while to fall in. | 10:16.42 |
| | I'm not sure image->color_params is ever used. | 10:16.57 |
| | In fact, I'm sure it's not. I'll just remove it. | 10:17.10 |
| | What is a proofing colorspace? | 10:17.47 |
| tor8 | that's me not knowing the terminology used here | 10:18.23 |
| | or the intent behind it | 10:18.30 |
| Robin_Watts | Right... you're asking the wrong guy to give you a definite answer, but as I understand it, it's used to transform the final result, | 10:18.36 |
| tor8 | oh, a *second* transform to get to 'proof'-view as rgb to show on a screen? | 10:19.12 |
| Robin_Watts | so you have an input colorspace, an output colorspace, and then the 'proofing' transform is used to simulate what it would look like on some other device. | 10:19.21 |
| tor8 | so you can spot clamping and banding errors, etc in cmyk | 10:19.30 |
| Robin_Watts | tor8: that sounds right. | 10:19.40 |
| tor8 | without having a cmyk display. right. | 10:19.43 |
| | does that really belong there though? I guess it might. | 10:19.55 |
| | I would instinctively have thought that would be part of the viewer :) | 10:20.10 |
| Robin_Watts | It's part of the draw device. | 10:20.46 |
| tor8 | right, close enough | 10:20.57 |
| | my curiosity is sated | 10:21.04 |
| Robin_Watts | and the draw device calls fz_paint_shade, so the info has to get into it somehow. | 10:21.10 |
| tor8 | let's skip lazy load pdf_document.oi for now | 10:21.54 |
| Robin_Watts | making the output intent lazy load... I asked mvrhel about that last night, and he wasn't sure how it could be more lazy. | 10:22.14 |
| tor8 | we load the output intent when we open the document | 10:22.39 |
| Robin_Watts | Or do you want to check whether we've loaded one, and load it on demand whenever we access oi ? | 10:22.41 |
| tor8 | I was thinking of loading on demand when we use it | 10:22.51 |
| Robin_Watts | We load oi, in pdf_load_default_colorspaces. | 10:23.11 |
| | oh, no we don't. | 10:23.24 |
| | That's when we set it. | 10:23.32 |
| | I see, so we'd load it as part of loading the first page, say | 10:24.17 |
| tor8 | oh, rats. mudraw calls the doc->get_output_intent function pointer directly | 10:24.31 |
| | without going through the fz_document_output_intent wrapper | 10:24.41 |
| | I'm not comfortable with the viewer needing to do stuff to get color management working like this | 10:25.21 |
| Robin_Watts | oh, ick, yes. | 10:26.08 |
| tor8 | oh, there is no wrapper! | 10:26.45 |
| Robin_Watts | That's the viewer saying "pick my output colorspace automatically". | 10:26.48 |
| tor8 | I'll add a wrapper and call it | 10:28.12 |
| Robin_Watts | ick. ick. ick. | 10:29.39 |
| | We are at pains in mudraw to set colorspace to be appropriate for the format we are writing in. | 10:30.11 |
| | And then this just overrides it to what the doc says. | 10:30.30 |
| | so if we are writing a pgm, and we get a document that has an RGB output intent, everything will fall to bits. | 10:30.58 |
| | We need some smarter logic there. | 10:31.11 |
| | only change the colorspace if a) it's not been specifically set, and b) it's one of the acceptable ones for this format. Otherwise print a warning and ignore it. | 10:31.45 |
| tor8 | it checks that the number of components match | 10:31.50 |
| Robin_Watts | Ah, ok. going blind, yes. | 10:32.02 |
| | I'm happier then. | 10:32.12 |
| tor8 | okay, two fixes on tor/lcms for the calxxx and output-intent loading | 10:35.35 |
| Robin_Watts | Urgh. horrible conflicts. How come? | 10:37.02 |
| tor8 | I edited the calxxx commits out of your commit and squashed them with my followup | 10:37.24 |
| | I changed "WIP: Review fixes for lcms branch." | 10:37.59 |
| Robin_Watts | tor8: right. | 10:38.03 |
| | yeah, that diff looks much saner. | 10:38.55 |
| | ok robin/lcms_squashed has this stuff all on now. | 10:40.31 |
| | (I'll copy it back to lcms and resquash etc at the end - this is just the branch I happen to be on) | 10:41.02 |
| tor8 | of course | 10:41.28 |
| Robin_Watts | I am tempted to rejig pdf_load_cal_common just a little more to make it more readable... | 10:41.51 |
| tor8 | okay, how? I think it's pretty straight forward now | 10:42.57 |
| Robin_Watts | tor8: tiny tiny change to avoid rightward drift. | 10:43.49 |
| | I thought you wanted pdf_load_cal_rgb to warn? | 10:44.05 |
| tor8 | the fz_throw error message is the 'warning' I meant, sorry. | 10:44.31 |
| Robin_Watts | does the throw actually get printed? | 10:44.49 |
| tor8 | on unixes it does | 10:45.04 |
| | might disappear (much like warnings) on win32 without an attached console | 10:45.20 |
| Robin_Watts | ok. | 10:45.37 |
| | ok, updated lcms_squashed. | 10:47.09 |
| tor8 | ah, fab! | 10:48.02 |
| | should've thought of that myself :) | 10:48.07 |
| Robin_Watts | pdf_load_default_colorspaces, last line... | 10:49.47 |
| tor8 | yes..? | 10:56.16 |
| Robin_Watts | tor8: sorry... | 10:59.15 |
| | penultimate line. | 10:59.23 |
| | that uses doc->oi | 10:59.29 |
| | which won't have done your lazy load thing. | 10:59.37 |
| tor8 | oh, crap. yes. | 11:00.43 |
| Robin_Watts | Also, how heavy is pdf_load_output_intent ? | 11:01.44 |
| | if there is no output intent in the file, then it'll get called every time. | 11:02.06 |
| tor8 | no heavier than pdf_load_colorspace | 11:02.10 |
| | it just looks at two or three PDF dictionaries looking for a special entry | 11:02.33 |
| Robin_Watts | yeah, is that heavier than we'd like? | 11:02.54 |
| | should we have a doc->oi_loaded flat ? | 11:03.02 |
| | flag | 11:03.04 |
| tor8 | so barely noticable, in respect to everything else page loading does | 11:03.05 |
| Robin_Watts | ok. | 11:03.09 |
| tor8 | calling it 10k times over a 10k page document would barely register :) | 11:03.30 |
| Robin_Watts | ok, so the fz_colorspace_is_pdf_cal one... I asked mvrhel last night and he said: | 11:05.10 |
| | I actually prefer pdf_cal since only pdf has those, but I can go either way. | 11:05.51 |
| | Do we believe there is ever going to be a case where they are used in another document format? | 11:06.43 |
| tor8 | I doubt it. but if we're going with pdf_cal then we should do so consistently. | 11:07.07 |
| Robin_Watts | tor8: where are we inconsistent? | 11:07.19 |
| tor8 | they could be used for PS should I get some massive brain trauma and decide to implement a PS interpreter :) | 11:07.28 |
| | we call them _cal colorspaces everywhere else | 11:07.38 |
| | fz_cal_colorspace, fz_new_cal_colorspace, etc | 11:08.12 |
| Robin_Watts | I agree with consistency :) | 11:08.34 |
| | which is the smaller edit? :) | 11:08.43 |
| tor8 | s/pdf_cal/cal/ | 11:08.49 |
| Robin_Watts | tor8: You do that, I'll do the color_converter change (to find and drop) | 11:10.13 |
| avih | tor8: i see you're busy so get to it when you can, but there's something extremely fishy with mujs numbers. this is reproducible on two OSs, exe bit deph, and [no/yes] optimization, in my program and also in mujs REPL (the paste is REPL): https://0x0.st/l0b.txt | 11:14.02 |
| | i suspect even a state corruption, but not sure | 11:14.38 |
| | (the number 3.599999999999999e15 is always the same) | 11:15.45 |
| Robin_Watts | The first one looks plausible. | 11:16.00 |
| | the second one does not :) | 11:16.06 |
| avih | sure, the first is correct | 11:16.09 |
| Robin_Watts | The third one could be a consequence of the second. | 11:16.18 |
| | avih: Stop breaking stuff :) | 11:16.47 |
| avih | yes, the 2ns and 3rd seem relatged, but still shouldn't yield different results | 11:16.50 |
| | related* | 11:16.59 |
| | heh | 11:17.01 |
| | and the 4th is out of the blue :) | 11:17.37 |
| Robin_Watts | I bet the 4th is related to the second too. | 11:18.47 |
| avih | as i said, i suspect some state corruption someplace, but not entirely sure. the 4th yields different results sometimes. but if you use a normal small number, like 4, then it yields the correct result repeatedly | 11:19.35 |
| Robin_Watts | 143 = 128+15 | 11:19.40 |
| avih | omg, and 42 = 6 * 7. you're a genius! :p | 11:20.08 |
| | (i fail to see the connection. yet.. :) ) | 11:20.26 |
| | well.. vaguely :) | 11:20.41 |
| Robin_Watts | well, 15 is the expected exponent, and exponents are 8 bit in floats etc. | 11:20.45 |
| avih | yeah. vague :) | 11:20.57 |
| Robin_Watts | so they are signed, so they can go from -128 to 127... | 11:21.02 |
| | so there is probably a +128 in there at some point... | 11:21.13 |
| avih | right. signed unsigned stuff | 11:21.15 |
| | but anyway, it seems this number confuses it quite a bit :) | 11:21.46 |
| | also how come each one gets a different value. i'd get wrong value repeatedly, but 4 different result.. it's something :) | 11:22.26 |
| tor8 | avih: that's .. weird stuff | 11:23.13 |
| | avih: try "var v = 3.599999999999999e15" without the semicolon | 11:23.24 |
| | print(v) prints 0 in that case | 11:23.32 |
| avih | and btw, i didn't engineer this number. i was testing stuff and got to it randomly | 11:23.33 |
| Robin_Watts | tor8: print(v); is the only one that's right! | 11:23.54 |
| | Don't try and break that one too :) | 11:24.02 |
| avih | now he killed it too :) | 11:24.08 |
| tor8 | valgrind goes nuts with "Conditional jump or move depends on uninitialised value" | 11:24.41 |
| | I expect some unbounded recursion stack smashing going on | 11:24.59 |
| Robin_Watts | or some running off the end of the buffer while parsing the exponent or something. | 11:25.19 |
| tor8 | yeat, stack corruption of some kind | 11:25.56 |
| avih | yeah | 11:26.04 |
| Robin_Watts | tor8: "why check ctx->colorspace" - good question. Possibly the cmm should be in the colorspace. | 11:27.57 |
| avih | fwiw, i got to it while writing a Number.toString pollyfill. i think it ended up quite useful, performance concerns aside: https://0x0.st/l0e.txt | 11:31.19 |
| tor8 | avih: testing calling js_strtod directly with various strings | 11:37.16 |
| | it seems to depend on what comes after the 3.599999999999999e15 | 11:37.29 |
| avih | after? | 11:37.38 |
| tor8 | if the string just ends or has a '\n' then we get an Out of range ERRNO | 11:37.42 |
| avih | you mean it does out of bounds access? | 11:37.46 |
| tor8 | if it ends with a ; then it returns Success and the 3.59e15 | 11:38.09 |
| | sorry, 3.6e9 same as libc strtod | 11:38.21 |
| avih | oh? but otherwise, yeah, it's just dangling of the edge of having enough precision digits | 11:38.50 |
| tor8 | I don't know why or how, but it seems to be affected by what terminates the number string | 11:39.02 |
| avih | (iirc 16-17 decimal digits) | 11:39.11 |
| | huh, that sounds a bug in the parsing then, isn't it? | 11:40.02 |
| tor8 | yes, it does | 11:43.56 |
| | d'oh. yes, massive typo in the exponent parsing code. | 11:48.56 |
| | if (c >= 0 && c <= '9') | 11:49.18 |
| | avih: okay, fix pushed | 11:52.33 |
| | Robin_Watts: yes, it seems a bit odd that the cmm is not part of the colorspace context | 11:52.55 |
| avih | lol | 11:52.56 |
| tor8 | avih: I think this warrants a new release | 11:53.26 |
| | but maybe I should get your radix toNumber stuff in first | 11:53.34 |
| avih | i tols you to tag it 0.8 or so :) | 11:53.41 |
| tor8 | a patch-release tag at least | 11:53.44 |
| avih | told* | 11:53.44 |
| tor8 | hey, this is part and parcel of what makes 1.0 so exciting ;) | 11:53.54 |
| avih | heh very true | 11:54.08 |
| | tor8: how annoying you think would be to write the radix thing in c with mujs APIs? did you consider implementing some functions in js? | 11:55.47 |
| tor8 | some functions would be much easier to implement in js | 11:56.09 |
| | but so far I've only resorted to doing so with 'require' | 11:56.21 |
| | implementing parts of the stdlib in js would bloat the memory use more, and slow down startup | 11:56.48 |
| | so I want to avoid it where reasonable | 11:57.01 |
| avih | granted, in c you have better use to the bits, if you know how to make use of it. for instance, my radix doesn't round, while firefox' or duktape's does (e.g. 35.999999999 to 36) | 11:57.12 |
| | (with enough digits, that is - that's where i encountered the issue btw. i did have rounding code first, but then decided it's too dangerous on edge cases and reverted to the trusty "render and then just stop render the digits") | 11:58.38 |
| tor8 | they should truncate, not round... | 11:58.44 |
| avih | that depends on the radix | 11:58.57 |
| tor8 | Number.prototype.toString radix should be read using the ToInteger primitive | 11:59.00 |
| | which is sign(number) * floor(abs(number)) | 11:59.23 |
| avih | yes, but when you render it as some radix it might be infinite decimal digits, so it can truncate or round the last digit | 11:59.32 |
| tor8 | yes, that's floating point parsing :) | 11:59.43 |
| avih | the thing is, it can affect all the digits of the number, like at the case i just posted | 11:59.54 |
| | as i said, i did have good rounding code, but decided it's too dangerous and not worth it. | 12:00.43 |
| tor8 | the ToInteger abstract operation is not exposed to the JS language | 12:00.46 |
| | totally not worth it | 12:01.05 |
| avih | doesn't parseInt effectively expose it? | 12:01.08 |
| tor8 | ToInteger maps a floating point number (as a double) to an integer (as a double) | 12:02.02 |
| avih | so is that a no? :) | 12:02.16 |
| tor8 | that is a no :) | 12:02.22 |
| avih | heh | 12:02.28 |
| | are you sure? double has at least 52 precision bits, while your ints are 32 bits, even if unsigned. where would it not be effectively "parseInt"? | 12:03.18 |
| tor8 | parseInt parses a string | 12:03.57 |
| | the rounding would be different based on the signedness | 12:04.22 |
| | or maybe not, I'm too brain fried by digging through this color management reviewing | 12:04.43 |
| avih | yeah, both can indeed fry your brain... | 12:05.03 |
| | tor8: so what's nice with my radix code is that it takes the precision bits into account. you could have zero or many many floating point [radix] digits depending on the size of the number. results are very close to what firefox produces on such cases, except that firefox rounds and i'm truncating | 12:06.54 |
| Robin_Watts | tor8: sorry, back from a washing machine related distraction. | 12:07.03 |
| | I'll move cmm into colorspace. | 12:07.09 |
| | That'll make stuff nicer, I think. | 12:07.16 |
| avih | e.g. the numbers 35.99999999999999 and 359999999999999.9 (same digits, different point position) tield the following numbers in radix 36: z.zzzzzzzzz and 3jlxpt2prz.v respectively, while firefox yields for the latter: 3jlxpt2prz.w (rounds) | 12:08.57 |
| | yields* | 12:09.07 |
| Robin_Watts | tor8: OK, lcms_squashed updated. | 12:15.07 |
| | I'm going to grab some lunch. | 12:15.13 |
| avih | tor8: that worked nicely. thanks :) | 12:28.11 |
| tor8 | avih: np. thanks for spotting the problem! | 12:35.46 |
| avih | tor8: btw, i noticed more than once already. at your commit messages, many times you don't what what was affected. i think it's really important that if people bump into bugs, they can check the log whether their use case seems affected by an existing fix | 12:36.31 |
| | don't write*. | 12:36.44 |
| | at this case it's not too bad since it states the subject, but still doesn't state what could have happened without the fix. on other cases, it's really hard what gets fixed when some internal function typo is fixed | 12:37.26 |
| tor8 | avih: right. I'm lazy when it comes to writing commit messages, and you're right that it would be better to point out some examples of what it fixes. | 12:38.04 |
| | often in mupdf we have a bug# to point to | 12:38.33 |
| avih | feel free to file bugs and refer to them :p or, easier, just add some more details at the commit message. you're good with words. c'mon :) | 12:39.25 |
| tor8 | I'll try to improve :) | 12:41.06 |
| avih | :) | 12:41.09 |
| | anyway, you're welcome, and thanks again for the quick response :) | 12:42.50 |
| Robin_Watts | tor8: another fix on lcms_squashed | 13:13.45 |
| tor8 | Robin_Watts: cool. those 3 lgtm. | 13:15.13 |
| | two more on tor/lcms | 13:15.19 |
| Robin_Watts | Conflicts resolved on lcms_squashed | 13:17.35 |
| | so "mirrors with inconsitent naming" | 13:18.20 |
| | just remove the fz_ ? | 13:18.58 |
| tor8 | yeah, I think that's enough | 13:19.05 |
| | the functions were a bit spread out so took me a while to find the pattern | 13:19.27 |
| Robin_Watts | Hmm. We have an fz_std_conv_pixmap too | 13:20.13 |
| tor8 | same thing there, drop the fz_+ | 13:20.47 |
| | ? | 13:20.47 |
| Robin_Watts | yeah, just doing that. | 13:21.09 |
| tor8 | PSD is photoshop files? | 13:21.50 |
| Robin_Watts | tor8: It is. | 13:22.39 |
| | yeah, bgr is broked. | 13:22.51 |
| tor8 | I guess generating them is a lot easier than reading them | 13:22.53 |
| Robin_Watts | tor8: reading them simply is easy enough (bmpcmp does it) | 13:23.15 |
| | reading them in full generality is harder. | 13:23.26 |
| tor8 | that's just reading the embedded preview bitmap though? I vaguely recall something like that. | 13:23.34 |
| | I'll fix fz_format_link_key | 13:23.49 |
| Robin_Watts | fz_md5_icc should just use fz_md5_buffer. Will fix. | 13:25.35 |
| | oh, ass. | 13:29.08 |
| | Moving cmm into colorspace has nasty problems in closedown. | 13:29.19 |
| | ok, not so nasty, just need to move a line. | 13:30.05 |
| tor8 | I forgot one line in the is_cal commit. fix on tor/lcms. | 13:30.41 |
| Robin_Watts | Cherry picked, squashed back, and on lcms_squashed. | 13:32.18 |
| | right, you wanted "clearer logic for fz_cmm_new_profile" | 13:32.52 |
| tor8 | I can do that, if you want | 13:33.01 |
| Robin_Watts | I already rejigged this code once to make it as clear as I felt it could be, so I think it's your turn :) | 13:33.22 |
| tor8 | okay :) | 13:33.29 |
| | Robin_Watts: we might want to memset profile->md5 to all 0 if there is no profile->buffer? | 13:34.21 |
| Robin_Watts | What does fz_md5_buffer do? | 13:34.49 |
| tor8 | not NULL safe | 13:35.06 |
| | it uses buffer->data and buffer->len | 13:35.12 |
| Robin_Watts | I think we should make it null safe. | 13:35.15 |
| tor8 | maybe that one should guard instead and do an md5 of the empty string | 13:35.27 |
| Robin_Watts | if (buffer) fz_md5_update | 13:35.28 |
| tor8 | yeah | 13:35.33 |
| Robin_Watts | that would then work with NULLs and with empty buffers. | 13:35.44 |
| | Urm... is it just me, or do we have 2 different fz_new_icc_data_from_icc_colorspace functions ? | 13:37.57 |
| tor8 | I only find one | 13:40.24 |
| Robin_Watts | oh, sorry. One is a cal not icc. | 13:43.44 |
| | And already uses a buffer. | 13:43.57 |
| | I'll make icc do the same. | 13:44.03 |
| tor8 | save_profile can be simplified to use fz_save_buffer | 13:45.20 |
| | I'll fix that too | 13:45.55 |
| | Robin_Watts: three commits on tor/lcms | 13:57.22 |
| Robin_Watts | tor8: Urm... first commit, change to colorspace.c... really? | 14:01.17 |
| | second commit, sorry. | 14:01.57 |
| tor8 | you mean splitting the if? | 14:02.00 |
| | the (cmm_handle == NULL && new_profile) pattern appears elsewhere | 14:02.14 |
| | I figured it would be more symmetric this way | 14:02.25 |
| | but really, should be baked into the third commit | 14:02.37 |
| Robin_Watts | tor8: yeah, why split the if? | 14:02.43 |
| tor8 | the "Check if correct type" case (as seen in the third commit) doesn't match the pattern though | 14:03.00 |
| | and the if clause for that comment seems to be baking in two tests -- creating the cmm handle, and testing the type | 14:03.33 |
| | "Create handle and check if correct type" maybe? | 14:03.52 |
| Robin_Watts | yeah. | 14:04.47 |
| tor8 | okay, updated commits on tor/lcms | 14:07.07 |
| | brb, I need to take a quick break | 14:08.06 |
| avih | tor8: btw, your fix seems to me like it's at the grisu2 code, which i believe is from some *BSD? if yes, is that an upstream bug? | 14:11.36 |
| tor8 | it is ancient bsd code, upstream has long since changed | 14:27.02 |
| | to deal with setlocale and other crap | 14:27.24 |
| | Robin_Watts: do we really need an 184k ICC file for CMYK? is there nothing smaller we can use? | 14:42.43 |
| Robin_Watts | tor8: That would be a question for mvrhel_laptop | 14:43.04 |
| | tor8: OK, so fz_new_icc_data_from_icc_colorspace and fz_new_icc_data_from_cal_colorspace | 14:47.22 |
| | The latter one *is* a new. | 14:47.29 |
| | cos it returns ownership of the buffer. | 14:47.50 |
| tor8 | I think we should either make fz_new_icc_data_from_icc_colorspace return a fz_keep_buffer(profile->buffer) | 14:48.01 |
| | or change the name | 14:48.03 |
| Robin_Watts | I *could* make fz_new_icc_data_from_icc_colorspace do a keep, but it would mean the caller had to do some try/catchery. | 14:48.31 |
| | I am more tempted to just change the name to be fz_icc_data_from_icc_colorspace | 14:48.50 |
| tor8 | yeah, that'll do fine. | 14:48.58 |
| Robin_Watts | Cool. | 14:49.06 |
| tor8 | Robin_Watts: a handful more commits on tor/master | 14:51.32 |
| Robin_Watts | tor8: in your java commit, the = 0; in the flags declaration is pointless. | 14:56.56 |
| tor8 | oh yeah. this is java. | 14:57.16 |
| Robin_Watts | tor8: no, it would be so in C too. | 14:57.25 |
| | cos the switch is exhaustive, and always sets flags. | 14:57.39 |
| tor8 | it would be, because of the default: but some compilers might complain anyway | 14:57.40 |
| | will fix | 14:57.51 |
| Robin_Watts | tor8: yeah, but sod them :) | 14:57.58 |
| | The makefile commit has a "foo:" target in that I bet was just for debug | 14:58.40 |
| tor8 | oops! | 14:58.53 |
| mvrhel_laptop | tor8: 184k is small. The actual swop icc cmyk profile is 545k I don't think you can cut it down much more without losing quality | 14:59.01 |
| Robin_Watts | tor8: Other than that lgtm. | 14:59.45 |
| tor8 | mvrhel_laptop: okay. how much quality would we lose if we could get it down to ~1k? could we replicate our old SLOWCMYK conversion? | 14:59.46 |
| mvrhel_laptop | ick | 14:59.55 |
| | you are not going to match adobe | 15:00.04 |
| tor8 | though I suppose having a 200k profile and getting good CMYK is a worthwhile tradeoff | 15:00.30 |
| | considering we embed some 20M of font data :) | 15:00.38 |
| | Robin_Watts: okay, updated commits | 15:01.05 |
| | and given those mistakes, maybe I should stop editing code now | 15:01.15 |
| Robin_Watts | tor8: New lcms_squashed | 15:02.05 |
| | I hate the vulkan-style idea. :/ | 15:02.51 |
| | So, returning to the stuff that we skipped.... | 15:04.05 |
| sebras | fyi: I just compiled tor/lcms in -mini and it compiled and ran fine. | 15:04.57 |
| | I was anticipating that something might have broken somewhere, but that was not the case! :) | 15:05.11 |
| tor8 | sebras: that bug we didn't fix but fixed, submodules in android viewers need updating | 15:05.31 |
| Robin_Watts | fz_cmm_instance - I'm happy to have that as a typedef struct fz_cmm_instance_s fz_cmm_instance; | 15:05.39 |
| | fz_ensure_pixmap_is_additive has a FIXME | 15:06.05 |
| sebras | ok.. I was fooled by git submodule update again. :-P | 15:06.06 |
| sebras | used to be so happy. | 15:06.26 |
| Robin_Watts | mvrhel_laptop: At various places we call fz_convert_pixmap() with NULL color_params. | 15:06.38 |
| | mainly cos we haven't got a non NULL color_params to pass in. | 15:06.58 |
| sebras | tor8: I know they need updating, but not what particular bug you are referring to. | 15:07.11 |
| Robin_Watts | We need to just double check them to be sure we are happy that that does the right thing. | 15:07.15 |
| | mvrhel_laptop: Can you spare some time to look over these with me? | 15:07.37 |
| mvrhel_laptop | Robin_Watts: sure. | 15:09.16 |
| Robin_Watts | Urgh. something broke with the icc_profiles.c change for windows. | 15:10.59 |
| tor8 | sebras: I can't remember which one. we fixed the bug and closed it, but the reporter was using android not desktop. | 15:11.19 |
| mvrhel_laptop | Robin_Watts: the places that deal with masks should just use null | 15:11.34 |
| | I would think | 15:11.51 |
| | hmm maybe I am misunderstanding though | 15:12.07 |
| Robin_Watts | fz_ensure_pixmap_is_additive is the big one. | 15:12.25 |
| mvrhel_laptop | hmm don't know if I ever saw that | 15:12.57 |
| | that is a weird operator | 15:13.23 |
| Robin_Watts | In fact, that may be the only one I care about. (pdfextract calls it too, but I don't care about that) | 15:13.29 |
| mvrhel_laptop | when would you use that | 15:13.31 |
| Robin_Watts | mvrhel_laptop: We hold alpha premultiplied. | 15:13.44 |
| | for pixmaps. | 15:13.50 |
| mvrhel_laptop | why would they need to be in an additive space? | 15:14.00 |
| Robin_Watts | So if we get an alpha pixmap with subtractive spaces, that's hard. | 15:14.02 |
| mvrhel_laptop | yes. but if you have a cmyk + alpha image that is color managed, going to RGB is not really the solution | 15:14.23 |
| Robin_Watts | mvrhel_laptop: mmm. | 15:14.31 |
| | how can we hold a cmyk + alpha image premultiplied ? | 15:14.54 |
| mvrhel_laptop | that is a good question. perhaps you can't. but converting the color space so that you can is really not a proper solution | 15:15.59 |
| | in terms of color managment | 15:16.10 |
| | all sorts of problems with that approach | 15:16.17 |
| | so the tiff spec allows cmyk + alpha | 15:16.47 |
| | is the data ever premultiplied? | 15:16.54 |
| Robin_Watts | It may be that we need to move to having pixmaps that can have non-premultiplied alpha (at least for non gray/rgb spaces) | 15:16.57 |
| mvrhel_laptop | Robin_Watts: yes | 15:17.09 |
| Robin_Watts | tor8: let's drag you into this discussion too. | 15:17.18 |
| | new lcms_squashed with various fixes. | 15:20.37 |
| | mvrhel_laptop: I suspect it's the plotters more than anything else that are affected by the use of non-premultiplied data. | 15:22.00 |
| | If everything gets converted down to greyscale or rgb before plotting, we're fine. | 15:22.38 |
| | How do we currently handle plotting an rgb+alpha image to a cmyk device? | 15:23.05 |
| mvrhel_laptop | Robin_Watts: are you asking me that | 15:24.27 |
| | ? | 15:24.30 |
| | Robin_Watts: So I don't quite understand why we can't have premultiplied CMYK data | 15:27.54 |
| Robin_Watts | mvrhel_laptop: I'm asking myself :) | 15:28.07 |
| sebras | Robin_Watts: what commit in thirdparty/lcms2 are you guys using? | 15:28.29 |
| Robin_Watts | sebras: see robin/lcms_squashed | 15:28.41 |
| | oh, sorry, in thirdparty/lcms2 it's thee mupdf_changes branch. | 15:29.15 |
| | 6b4828a5c9156c2f7a656e69a20ec3d760635c0b | 15:29.32 |
| mvrhel_laptop | Robin_Watts: along these same lines, I see that all the blending in mupdf assumes RGB data | 15:29.33 |
| | ? | 15:29.35 |
| sebras | thanks. | 15:29.38 |
| Robin_Watts | mvrhel_laptop: Yes. | 15:29.43 |
| | All the blending assumes RGB. | 15:29.50 |
| mvrhel_laptop | ok. I will work on fixing that today | 15:29.55 |
| | I have the color spaces and conversions working with the group push and pops | 15:30.15 |
| | but I did not foresee this blending issue | 15:30.31 |
| Robin_Watts | so, if I have bytes c,m,y,k,a... does that mean that the actual color is c/a, m/a, y/a, k/a ? | 15:31.02 |
| | (where the bytes are premultiplied) | 15:31.32 |
| | mvrhel_laptop: We have a showstopper stopping us getting the lcms branch in. | 15:31.54 |
| | Namely that we don't support bgr. | 15:32.02 |
| mvrhel_laptop | Robin_Watts: so I was simply thinking of this in terms of image data that ranges from 0 to 255 and an alpha value. | 15:32.16 |
| | really the cmyk and rgb pre-multiplied problems are the same | 15:32.35 |
| Robin_Watts | The windows port relies on bgr. | 15:32.47 |
| mvrhel_laptop | you just want to get back and forth between the two | 15:32.51 |
| | ok bgr | 15:32.56 |
| Robin_Watts | Let me try the extremely scientific method of removing the ensure_premultiplied_alpha calls and seeing what breaks :) | 15:33.30 |
| mvrhel_laptop | I really dislike having that as color space in mupdf | 15:33.49 |
| Robin_Watts | or maybe I should do that AFTER lcms goes in, cos that'll reduce the number of diffs. | 15:33.51 |
| | mvrhel_laptop: Why? | 15:34.06 |
| mvrhel_laptop | because I think internally we should keep things as simple as possible and handle it on the output end (and I guess if we are dealing with bmp input doing it there too). I guess what we will need to do is add it as a formatter option in lcms | 15:36.21 |
| | Robin_Watts: brg | 15:38.12 |
| | brb | 15:38.14 |
| Robin_Watts | mvrhel_laptop: brb would be a really crap colorspace. | 15:38.30 |
| mvrhel_laptop | ha | 15:38.42 |
| Robin_Watts | mvrhel_laptop: Yes, but there are lots of devices out that that require bgr format data. | 15:38.58 |
| | and the benefits of being able to output direct into a memory block of the right format justify the added complexity, I feel. | 15:39.26 |
| | (windows machines for one) | 15:39.31 |
| sebras | Robin_Watts: sebras/lcms is based on tor/lcms and has a patch needed for lcms2 to compile successfully in android. | 15:44.53 |
| | Robin_Watts: running the app crashes in fz_new_rasterizer() because aa supplied by fz_new_draw_device() is unconditionally NULL. did I do something wrong? | 15:45.48 |
| Robin_Watts | sebras: Urm... | 15:46.53 |
| sebras | I crash on the ctx->aa->bits comparison in fz_new_rasterizer(). | 15:46.59 |
| Robin_Watts | ctx->aa is NULL ? | 15:47.37 |
| sebras | Robin_Watts: yes. | 15:47.56 |
| Robin_Watts | OK, so code in that function is clearly wrong, but ctx->aa shouldn't be NULL. | 15:48.07 |
| sebras | Robin_Watts: the imput argument aa is also NULL. | 15:48.08 |
| Robin_Watts | Ah, are you building with AA_BITS predefined? | 15:48.46 |
| mvrhel_laptop | Robin_Watts: ok back . sorry | 15:49.18 |
| Robin_Watts | mvrhel_laptop: No worries. I'm going to leave the premultiplied alpha stuff until after we get lcms in. | 15:49.38 |
| mvrhel_laptop | Sounds like a good idea | 15:49.48 |
| | I can work on getting bgr formatting support into lcms I guess | 15:50.03 |
| Robin_Watts | cool. let's have a look at how hard that will be... | 15:50.32 |
| mvrhel_laptop | I need to think a bit about that | 15:50.40 |
| sebras | Robin_Watts: yes, I think Android.mk supplies -DAA_BITS=8 | 15:51.04 |
| Robin_Watts | sebras: Right. Let me get you some fixed code. sorry. | 15:51.24 |
| sebras | Robin_Watts: np. | 15:52.50 |
| Robin_Watts | mvrhel_laptop: looks like lcms already support bgr in some formatters. | 15:53.03 |
| | TYPE_BGR_FLT, TYPE_BGRA_FLT etc | 15:53.18 |
| sebras | Robin_Watts: if my stuff is distracting you, let me know. | 15:53.30 |
| mvrhel_laptop | ah TYPE_BGR_8 | 15:54.31 |
| Robin_Watts | sebras: No, this is a good time, I think. | 15:55.36 |
| mvrhel_laptop | Robin_Watts: so here is the question | 15:55.40 |
| Robin_Watts | oh, oh, famous last words :) | 15:55.54 |
| mvrhel_laptop | Lets say you want to go to a bmp output format that needs bgr. It still is going to use the sRGB or DeviceRGB ICC profile. We need to add some information someplace that lets us know that this color space is actually BGR. So that when lcms does its mapping it knows to pick the TYPE_BGR_8 | 15:57.21 |
| | for the output. (Or for the input if the input is a bmp file with bgr data) | 15:57.46 |
| Robin_Watts | Can we not have a BGR ICC Profile? | 15:59.24 |
| mvrhel_laptop | no | 15:59.36 |
| Robin_Watts | Why not ? | 15:59.44 |
| mvrhel_laptop | not really | 15:59.44 |
| | well we could make an ncolor profile | 15:59.56 |
| | that had a reorder of the data | 16:00.06 |
| Robin_Watts | The ICC Profile format doesn't know about BGR, only RGB ? | 16:00.17 |
| mvrhel_laptop | right | 16:00.21 |
| Robin_Watts | And would a ncolor profile do the same thing in all circumstances | 16:00.42 |
| | ? | 16:00.43 |
| mvrhel_laptop | an ncolor profile can map out to any colorants | 16:01.08 |
| | This is not the way to go | 16:01.17 |
| | for example, a bmp file which has data in BGR order would not have a ncolor icc profile in it | 16:01.43 |
| | it would have an RGB profile in it | 16:01.49 |
| Robin_Watts | mmmm. | 16:02.40 |
| | Do you get ICC profiles in bmp files? | 16:03.02 |
| mvrhel_laptop | yes | 16:03.09 |
| | the format supports icc profiles in the header I believe | 16:03.24 |
| | So in mupdf lets look where BGR data is going to be encountered | 16:04.00 |
| Robin_Watts | is bgr rather than rgb (for output at least) something that could be done with the proofing profile? | 16:04.06 |
| mvrhel_laptop | Robin_Watts: no and there are other issues with that regard. | 16:05.52 |
| | in terms of say a PDF input file that has an output intent that was RGB based | 16:06.21 |
| | and you are going out to a BGR device | 16:06.26 |
| | doing your approach of the BRG (ncolor) ICC profile is tricky there due to how we decide to swap out the profile | 16:07.11 |
| | Anyway, lets back up for a second and look where we will encounter BGR in mupdf | 16:07.34 |
| Robin_Watts | mvrhel_laptop: OK. Can we have an extra bit in the colorspace to say "I am bgr" and use that to set the right formatter? | 16:08.00 |
| | sure, let's do that. | 16:08.06 |
| mvrhel_laptop | Robin_Watts: yes, that was my thought | 16:08.13 |
| | but I did not know if you or tor would not like that | 16:08.22 |
| | Its not the most elegant | 16:08.43 |
| Robin_Watts | Currently we have different 'to_rgb' and 'from_rgb' functions, IIRC. | 16:09.03 |
| mvrhel_laptop | Those should have been replaced with to_ccs and from _ccs | 16:09.27 |
| Robin_Watts | yes, I mean pre-lcms. | 16:09.33 |
| sebras | Robin_Watts: if I change Android.mk to not set AA_BITS it not longer crashes. and I have rendered pdf. | 16:09.35 |
| Robin_Watts | sebras: cool. I have a quick fix here, but I'd like to test it a bit. | 16:09.51 |
| | can you bear with me for a bit ? | 16:09.56 |
| sebras | Robin_Watts: np. | 16:10.07 |
| Robin_Watts | sebras: Sleep, you need sleep... | 16:10.27 |
| sebras | Robin_Watts: I have a test tomorrow too. | 16:10.38 |
| Robin_Watts | sebras: ç¡è§ | 16:11.53 |
| | mvrhel_laptop: So, we need bgr data out for the windows viewer app. | 16:13.02 |
| | We need bgr data out for the TGA format. | 16:13.19 |
| | We need bgr data out for java format bitmaps. | 16:13.43 |
| | (Android for one) | 16:13.49 |
| | For windows and TGA we could do a post render phase to swap stuff, but really we don't want to be doing that for android, cos CPU cycles are scarce enough. | 16:14.42 |
| mvrhel_laptop | Robin_Watts: I understand. I think having a bit set that lets lcms know that this RGB color space is really BGR should do the trick | 16:16.24 |
| Robin_Watts | For bmp input, sebras appears to convert to rgb. | 16:16.25 |
| mvrhel_laptop | oh good | 16:16.31 |
| Robin_Watts | So yeah, let's just do the nice simple extra bit. | 16:16.47 |
| mvrhel_laptop | I am just trying to think if there are any transparency blending issues | 16:17.06 |
| | with groups | 16:17.09 |
| Robin_Watts | None of the blending should matter, cos the channels are all treated equivalently, right? | 16:18.28 |
| mvrhel_laptop | I can't think of a case that will be a problem. But in a way that bothers me | 16:18.51 |
| | ok. Lets try it and see. Do you want to do the change? | 16:19.53 |
| sebras | Robin_Watts: ç¡è§ï¼é£æ¯ç°¡é«åï¼æå¸ç¹é«åï¼æä»¥ä½ æ該寫âç¡è¦ºâã | 16:19.58 |
| mvrhel_laptop | or do you want me to | 16:20.00 |
| Robin_Watts | Can you handle this one? | 16:20.13 |
| mvrhel_laptop | Robin_Watts: yes | 16:20.28 |
| Robin_Watts | mvrhel_laptop: Thanks. | 16:20.42 |
| mvrhel_laptop | Robin_Watts: what should I work off of? | 16:20.48 |
| sebras | if you guys feel you need to change the bmp stuff, feel free do to so, though it seems like you are already happy about it. | 16:20.49 |
| Robin_Watts | sebras: We are indeed happy with it. | 16:20.58 |
| mvrhel_laptop | sebras: no we want RGB | 16:21.00 |
| | or I do | 16:21.04 |
| Robin_Watts | mvrhel_laptop: If we DID get a bgr bitmap in from somewhere, that wouldn't matter though, right? | 16:21.27 |
| | If someone uses the java bindings for MuPDF and calls in with a pixmap from java, that would be in bg. | 16:21.50 |
| mvrhel_laptop | Robin_Watts: as long as the color space is labeled with the magic bit, we should be fine | 16:21.51 |
| Robin_Watts | If someone uses the java bindings for MuPDF and calls in with a pixmap from java, that would be in bgr. | 16:21.52 |
| | Yes, it would be tagged as being fz_device_bgr | 16:22.11 |
| mvrhel_laptop | Then we should be good | 16:22.17 |
| Robin_Watts | sebras: å»ç¡è§ then. | 16:23.35 |
| sebras | Robin_Watts: 好ï¼æå®ã | 16:24.12 |
| mvrhel_laptop | Robin_Watts: you and tor8 have made a lot of changes. what should I work off of | 16:24.34 |
| Robin_Watts | mvrhel_laptop: Work off robin/lcms_squashed. | 16:24.57 |
| mvrhel_laptop | ok thanks | 16:25.02 |
| Robin_Watts | and I'll pull it all back. | 16:25.05 |
| | mvrhel_laptop: actually... | 16:25.24 |
| | if it's not to late, work off whatever you have that works. | 16:25.32 |
| | and I'll worry about resolving it. | 16:25.39 |
| | I've hit some problems here that I'm fixing in robin/lcms_squashed. | 16:25.54 |
| | and you shouldn't have to deal with those. | 16:26.02 |
| mvrhel_laptop | Robin_Watts: ok. | 16:26.18 |
| avih | tor8: so was the bug at the original code you copied? i assume you didn't type it in yourself, right? | 16:28.07 |
| Robin_Watts | avih: I believe we've modified that code significantly from the original. | 16:28.44 |
| avih | oh, hmm.. interesting. thx. | 16:28.59 |
| Robin_Watts | mvrhel_laptop, tor8: OK, so I've rejigged everything onto lcms, and lcms_squashed | 17:32.18 |
| mvrhel_laptop | ok | 17:32.32 |
| Robin_Watts | mvrhel_laptop: When you get the bgr stuff done, I'll pull that in too, and I hope we should be done. | 17:33.10 |
| mvrhel_laptop | cool | 17:33.33 |
| Robin_Watts | now let me look at the cluster stuff.... | 17:33.55 |
| tor8 | avih: I pulled the code from an old ruby release | 18:40.36 |
| | Robin_Watts: a bit busy, will look at the discussion above later | 18:41.13 |
| Robin_Watts | tor8: New lcms and lcms_squashed. | 18:41.13 |
| | tor8: No worries. Suffice to say, I think it's all there now. Have a good night. | 18:41.29 |
| mvrhel_laptop | Robin_Watts: so where and when is fz_blend_pixel called? | 21:29.54 |
| | I can't seem to see it used ever | 21:30.02 |
| | Robin_Watts: tomorrow I want to chat with you a bit about the deviceN stuff. I want to add a bit more information into fz_colorspace which I will need to handle things properly in the transparency code | 21:53.20 |
| | This is all going to get a little interesting and likely break some assumptions in the code | 21:54.22 |
| Robin_Watts | sure. | 22:30.24 |
| | fz_blend_pixel is never called as far as I can see. | 22:31.24 |
| | fz_blend_pixmap is the place where the magic happens. | 22:32.29 |
| | and that calls out to fz_blend_{non,}separable{_nonisolated,} | 22:33.35 |
| | That all uses repeated invocations of static inline code to do templated code, so it should be fairly efficient. There is scope for further unrolling in need be, but this isn't a huge hotspot in most stuff. | 22:35.25 |
| mvrhel_laptop | Robin_Watts: ok that is what I thought | 22:50.31 |
| | Robin_Watts: so there will likely be some spots for speed improvement later in all of this. I am working toward getting it correct first | 22:51.10 |
| | Robin_Watts: we can talk tomorrow more. | 22:51.21 |
| | i know its late there | 22:51.26 |
| | hmm I am a little confused on how mupdf works with respect to transparency. | 23:33.41 |
| | need to take a break for a bit | 23:38.02 |
| | Forward 1 day (to 2017/06/22)>>> | |
Log of #mupdf at irc.freenode.net.