| | <<<Back 1 day (to 2017/04/06) | 20170407 |
| DeadManMoshing | Good we hour morning(for me anyway) | 06:38.25 |
| ray_laptop | amila: I would add to kens' response about description of source files: Our convention is that each file has a one line comment after the Copyright header that explains what the file is, in general. Since this is a single line, it is very general | 13:07.04 |
| half-a-pony | ping anyone ( kens ?)? I was wondering if it would be feasible to disclose (publicly or privately) the reproducer for https://bugs.ghostscript.com/show_bug.cgi?id=697457 (in comment #1). that would be helpful for verification | 13:09.27 |
| kens | half-a-pony : let me look | 13:10.24 |
| half-a-pony | thx! | 13:10.42 |
| kens | Do you really need the file to reproduce ? I believe its already fixed | 13:11.08 |
| half-a-pony | kens: yah, that would be for checking which RHEL versions would be affected, ensure the backport is correct, etc. | 13:11.57 |
| kens | Although the fix is in the current release, there are still an awful lot of people and distributions using old versions of Ghostscript, so I'm a little wary about exposing it. OTOH we've been less careful in the apst | 13:12.26 |
| | half-a-pony : can you give me some kind of identity ? (are you the front or back half by the way ;-) | 13:12.50 |
| | I'd suggest all current versions of RHEL are affected, I don't think that's been updated to the current code | 13:13.18 |
| half-a-pony | :D :D the front half, I think ... though first time I got the question | 13:13.23 |
| kens | Are you some kind of maintainer for RHEL though ? | 13:13.52 |
| half-a-pony | kens: well, to assert my identity : you can just send it by e-mail to secalert@redhat.com .. I think it's the best way to assert I m from the redhat sec team | 13:14.22 |
| kens | From what I see of the patch, the commit that fixed it was large, and affected jbig2dec onliy | 13:14.35 |
| | OK well I can mail the file there. | 13:14.43 |
| amila | i saw ray. so i start to read file by file and i'm doing a mini manual for me | 13:16.12 |
| half-a-pony | kens: thanks! | 13:18.05 |
| | kens: I m a bit of a ghostscript n00b' still, but ghostpdl would be affected unless it's specificly compiled without jbig2dec, right ? | 13:19.30 |
| kens | half-a-pony : it *may* be affected. TO be honest, we didn't check that. | 13:20.12 |
| | Both Ghostscript and MuPDF use jbig2dec though | 13:20.25 |
| | You will need to build the jbig2dec executable to test the file, neither Ghostscript nor MuPDF will read a .jb2 file as is (at least I don't *hitnk* MuPDF will, I might be wrong there) | 13:21.15 |
| half-a-pony | kens: :D yah, my plan was that once I get the reproducer to work against jbig2dec, then I need to figure how to embed that in a pdf/ps | 13:22.57 |
| kens | half-a-pony : the email is on its way. | 13:23.06 |
| | half-a-pony : you can't put a JBIG2 image in a PostScript file, PostScript deosn't support JBIG2. You'd have to use a PDF file | 13:25.49 |
| half-a-pony | kens: oh! thanks! I ll only go through pdf then ... as I said, I m still learning this part | 13:27.03 |
| kens | NP | 13:27.11 |
| ray_laptop | half-a-pony: except with Ghostscript -- assuming it's a standard build that supports PDF and PS, then JBIG2Decode is available to PS: (my.jb2) (r) file /JBIG2Decode filter | 14:09.23 |
| | will create a PS "file" object that is read then passed through the JBIG2 decoder | 14:10.21 |
| | half-a-pony: but PDF is probably less work | 14:11.03 |
| | trying to fix up the tags, I am tripping over grestore -- it resets the pgs->color[0].dev_color to a value that may not have the tag set, but doesn't change the dev->graphics_type_tag | 14:14.00 |
| half-a-pony | ray_laptop: yah, I guess I need to try only 1 of them ... and PS doesn't look simple indeed :D | 14:15.10 |
| | kens: btw, it seems that no mails were received on our end yet :( | 14:16.08 |
| kens | Hmm | 14:16.14 |
| | let me check | 14:16.18 |
| | ah bum Eudora has been sitting on it, just a mo | 14:16.32 |
| ray_laptop | I don't really want to add the graphics_type_tag to the gstate (although that is one solution), but how bad would it be to have grestore set the tag to UNTOUCHED or UNKNOWN so that places that want to "ensure_tag_is_set" can update the dev->graphics_type_tag and set the dev_color correctly | 14:16.53 |
| kens | half-a-pony : OK it really has gone now | 14:17.21 |
| ray_laptop | kens: you may want to weigh in since you are mucking about with the gstate currently | 14:17.21 |
| kens | ray_laptop : I don't really understand the problem | 14:17.32 |
| | I thought you were putting the tag into the colour | 14:17.44 |
| ray_laptop | let me explain a bit more (and mvrhel may weigh in when he shows up). | 14:18.08 |
| | the tag value in dev->graphics_type_tag gets munged into the dev_color. for most (normal) devices, this is done by the dev-procs.encode_color but this *only* gets called if the pgs->color[0].dev_color->type is gx_dc_type_none which implies that dev_color needs to be set | 14:20.15 |
| | this is baked into gx_set_dev_color | 14:20.36 |
| | but when the color value is initially set (e.g. by setrgbcolor, it sets the dev_color, and the tag at that time will be whatever was last in the dev->graphics_type_tag (UNTOUCHED? ...) | 14:22.34 |
| kens | unknown sounds more reasonable | 14:23.31 |
| ray_laptop | so if we do a gsave, then fill (this sets the tag to PATH and does set_color which encodes PATH into the dev_color), then grestore (which sets the dev_color to whatever was gsaved). The next fill looks at the dev->graphics_type_tag which was *NOT* touched by grestore and so thinks the dev_color is copacetic | 14:25.46 |
| half-a-pony | kens: receieved, thanks! | 14:26.23 |
| kens | Ah good, sorry for the delay | 14:26.33 |
| | ray_laptop : it seems to me that the 'tag' is not part of the graphics styate at all, its related solely to the operation., but perhaps I'm misunderstanding | 14:27.49 |
| | If I set a colour and then do a fill, then the tag is fill. If I then do a stroke, then the tag is stroke. | 14:28.07 |
| | These are related ot the operation type, not the graphics state, surely | 14:28.20 |
| ray_laptop | so I can either have grestore invalidate the dev_color (which will cause set_dev_color to re-encode the color), or set the graphics_type_tag to UNToUCHED or UNKNOWN (anything other than TEXT, IMAGE, PATH) or, if the gstate included the tag that was in the device, it could "restore" that to the device upon grestore | 14:28.30 |
| kens | Give me a minute, I'm still replying tothe customer | 14:29.48 |
| ray_laptop | the problem is at the end of a fill or stroke or image or text, it doesn't reset the graphics_type_tag to whatever it was upon entry. This is because set_dev_color is sort of expensive and if the color+tag didn't change, we don't want to remap the color | 14:30.26 |
| | kens: np | 14:30.32 |
| | so that subsequent operations of the same type (tag) with the same color (fills, text, whatever) don't have to remap the color | 14:31.32 |
| Robin_Watts | ray_laptop: It seems to me that if the dev_color is dependent on the tag, and the dev color is affected by gsave/grestore, then the tag really ought to be too. | 14:35.01 |
| kens | ray_laptop : so we only change the tag type if the operation and current tag type do not match, is that correct ? | 14:35.08 |
| Robin_Watts | Otherwise we break the contract that gsave/grestore will result in consistent states. | 14:35.47 |
| kens | well it depends on how you view the tag | 14:37.15 |
| | If the tag is part of the graphics state, then gsave/grestore need to preserve it | 14:37.31 |
| | But it seems to me its *not* part of the graphcis state, because I can use the same colour once for a fill and again for a stroke, and the only thing that changed was the oepration, not the graphics state | 14:38.01 |
| | But the tag has to change | 14:38.09 |
| | WHich isn't a reason not to put it in the graphics state, but I can't really see why we need it | 14:38.38 |
| | How do we know, when performing an image/text/vector operation whether the tag type is currently correct ? | 14:39.44 |
| ray_laptop | kens: sorry -- was on the other channel | 14:42.11 |
| | kens: yes, we only change (set_graphics_type_tag) if the current op and dev->graphics_type_tag differ | 14:43.17 |
| | in which case we also unset the dev_color -- this is now baked in the ensure_tag_is_set macro | 14:43.47 |
| chrisl | ray_laptop: after a grestore, how do we know when we need to do a set_dev_color() 'cause the color has changed? | 14:44.05 |
| ray_laptop | chrisl: grestore restores the dev_color | 14:44.50 |
| | to whatever was gsaved | 14:45.03 |
| kens | Hmm, but how do you know ehn the next operation takes place whether you need to set the dev_color ? | 14:45.34 |
| chrisl | It does sound, to me, like the tag should be in the graphics state | 14:47.03 |
| ray_laptop | that's sort of the problem. If the dev_color was "set" with one tag, then gsave, then set a new tag we set the dev_color with the new tag, but grestore goes back to the dev_color with the original tag | 14:47.09 |
| kens | Carrying around the tag in the graphics state doesn't seem to help, if we did : text gsave image grestore vector, it doesn't matter whether the tag in the original gstate was 'text' or whether the encoded color was encoded with 'text'. The two seem the same to em, but I don't really understnad this stuff | 14:47.13 |
| | ray_laptop : yes, but why is that a problem ? And how does carrying around the tag in the gstate help ? | 14:47.38 |
| ray_laptop | the "color" part of the dev_color is still valid after the gsave set_dev_color grestore but the tag part isn't | 14:47.47 |
| kens | Surely its still 'valid', it says 'the last thing I did was an moperation of type (whatever)' | 14:48.27 |
| | So if I do another operation of the same type its OK, and if its a different type, then I need to change it | 14:48.54 |
| ray_laptop | if the gstate has the tag, then grestore can check dev->graphics_type_tag and update it to the saved tag (call dev->proc.set_graphics_type_tag) if needed | 14:49.04 |
| chrisl | I think what ray_laptop means is that the tag value in the device is now out of sync with the tag value encoded in the dev_color | 14:49.14 |
| | (potentially) | 14:49.33 |
| ray_laptop | kens: then the op will check the current dev tag find it the same and won't need to do anything | 14:49.46 |
| | chrisl: yes | 14:49.57 |
| kens | I still don't see the problem :-( | 14:50.01 |
| | If I do text then the tag goes to text. I do a gsave athen image, the tag goes to image | 14:50.17 |
| | I do a grestore | 14:50.24 |
| | The tag is now 'image' | 14:50.28 |
| | So if I do an image I don't need to change the tag, if I do something else I do need to change it | 14:50.43 |
| chrisl | Which tag? The one in the device, or the one in the dev_color? | 14:50.52 |
| kens | THe on in dev_color | 14:50.59 |
| | Why do we have two ? | 14:51.04 |
| | Surely the tag in dev_color is what we need | 14:51.16 |
| chrisl | Erm, that I can't answer...... | 14:51.32 |
| | ray_laptop: isn't there a fairly easy way to compare the tag value encoded in the dev_color with the tag value in the device? | 14:52.11 |
| ray_laptop | kens: the problem showed up when I changed fillpage to set UNTOUCHED. fillpage, set color (dev_color set with UNTOUCHED tag), gsave, fill (tag and dev_color set with PATH), grestore, fill | 14:52.13 |
| | after that the fill has the wrong tag part of the dev_color (graphics_type_tag still at PATH, but dev_color set with UNTOUCHED) | 14:52.56 |
| kens | so why can't you compare graphicsd_type_tag with teh dev_color to see if they are not the same ? | 14:53.50 |
| ray_laptop | chrisl: we would need a "get_graphics_type_tag" dev->proc (or some special knowledge of how the tag is encoded/stored in the dev_color) | 14:54.00 |
| kens | Well a spec_op would seem like an easier way ot do that | 14:54.23 |
| chrisl | isn't that what decode_color is for? | 14:54.39 |
| kens | I will admit that ot me it makes little difference, adding more stuff to the graphics state makes no odds to what I'm doing | 14:54.44 |
| ray_laptop | kens: the dev_color is one of many types -- for pure, it is put in by the dev->encode_color for devn or other ??? | 14:55.02 |
| kens | Hmm, didn't htink of decode_color | 14:55.06 |
| | ray_laptop : I don't understnad anything about the dev_color stuff at all, except to depsise it | 14:55.23 |
| | But as chrisl says, why can't decode_color giv eyou the information ? | 14:55.46 |
| ray_laptop | chrisl: decode_color doesn't return the tag, and decode_color is only valid for "pure" (gx_color_index) colors | 14:55.47 |
| kens | Or is that expensive too ? | 14:55.52 |
| kens | boggles | 14:56.33 |
| | so encode/decode are not symmetric ? Or did I misundertand that ? | 14:56.46 |
| ray_laptop | decode_color can be expensive: decode_color (usually calls map_color_rgb) is not expected to be frequent and not very optimized | 14:56.50 |
| | I wasn't part of the encode/decode color stuff (that was Raph and Dan, iirc) | 14:57.42 |
| | and it totally doesn't work with devn devices (hl_color) | 14:58.12 |
| chrisl | Shouldn't we fix that, then?? But that's another issue....... | 14:59.13 |
| kens | Well, I don't know enough about the encode colour stuff to say much, I can't really see why we need to cary round yet another copy of the tag, but if we do we do.. Adding yet more stuff to the graphics state isn't particularly a problem for me and its so vast already another byte won't make any difference. | 15:00.09 |
| ray_laptop | but for the bitrgbtags device (our only example) decode_color only returns the RGB values -- implied by num_components = 3 (num_components does NOT include the tag) | 15:00.10 |
| chrisl | I'm not sure I see a good solution to this: if we move the tag from the device to the gstate, it might end up not being available everywhere it's currently used | 15:00.53 |
| ray_laptop | kens: we _could_ as I mentioned toward the start, have grestore "invalidate" the dev->graphics_type_tag by setting it to UNTOUCHED or UNKNOWN | 15:01.39 |
| kens | ray_laptop : yes but that means we would immediatley have to perform an (exepnsive) operation, even if we didn't actually need to | 15:02.04 |
| | And gsave/grestore is a very common operator | 15:02.17 |
| ray_laptop | this would then not match the tag on any subsequent marking op, so an extra gx_set_dev_color will occur | 15:02.22 |
| kens | If it were not for the possible performance penatly (on all files and devices apparently?) I'd just invalidate it, but I'm worried this will be expensive | 15:03.08 |
| ray_laptop | but compared to gsave/grestore, gx_set_dev_color is not all that "expensive" (although it can involve an ICC transform) | 15:03.12 |
| | for instance, if we had gsave setcolorspace grestore, that is much more expensive | 15:04.32 |
| kens | gsave/grestore round paths is much more common | 15:05.01 |
| | and so ...path... gsave stroke grestroe fill would get an extra unneeded set_dev_cvlor | 15:05.34 |
| | But I don't know how bad this performance implication is. | 15:05.53 |
| ray_laptop | kens: right -- to avoid the implicit newpath -- and the color is usually not changed | 15:05.56 |
| kens | If you think the penalty is low, then I'd be inclined to just invalidate on grestore. If you think its significant, then maybe its better to store the tag | 15:06.41 |
| ray_laptop | kens: I suppose I could just put that simple example in a loop and try it with and without the "invalidate" in grestore | 15:07.01 |
| | kens: I'll try that | 15:07.14 |
| kens | OK maybe getting a figure is the best solution | 15:07.23 |
| ray_laptop | I have to run an errand. bbiab. I'll have performance #'s when I get back. | 15:07.49 |
| | kens: mvrhel_laptop: so invalidating the graphics_type_tag unconditionally and running: .1 .2 .3 .4 setcmykcolor 10 setlinewidth 1000000 { 100 100 20 20 rectfill gsave fill grestore stroke } repeat | 16:26.54 |
| | showpage | 16:26.55 |
| | goes from 1.626 seconds up to 2.008 sec (24% longer). What I added to gs_grestore_only was: dev_proc(pgs->device, set_graphics_type_tag)(pgs->device, GS_UNKNOWN_TAG); | 16:26.57 |
| mvrhel_laptop | wow | 16:27.33 |
| kens | that's bad | 16:28.02 |
| ray_laptop | I chose cmykcolor because I had an RGB device and wanted to make it do the ICC conversion :-) | 16:28.58 |
| | running it with .1 .2 .3 setrgbcolor gets it (mostly) back to 1.659 sec (note cmykcolor with -dUseFastColor is 1.718 sec) | 16:31.41 |
| | so it shows why we try to avoid color remapping | 16:32.22 |
| | if we put the tag into the dev_color struct (there are two, one for color[0], one for color[1] the ensure_tag_is_set could check the condition (dev->graphics_type_tag == dev_color.tag | 16:36.32 |
| | the dev_color _is_subject to gsave / grestore | 16:36.55 |
| | kens: what do you think about that? | 16:37.35 |
| | oops | 16:37.42 |
| | mvrhel_laptop: any opinion ? | 16:38.39 |
| | kens: what do you think about that? | 16:39.26 |
| kens | I don't know its not in the logs yet | 16:39.40 |
| ray_laptop | kens: (you went away just as I asked) | 16:39.44 |
| kens | I don't have a strong opinion on where to store the tag, the dev_color struct seems reasonable | 16:41.17 |
| ray_laptop | kens: ok, thanks. | 16:42.51 |
| | Forward 1 day (to 2017/04/08)>>> | |
Log of #ghostscript at irc.freenode.net.