| | <<<Back 1 day (to 2018/04/24) | 20180425 |
| HenryStiles | sebras: I'm afraid you've modified jbig2 to the point of ownership (http://git.ghostscript.com/?p=user/sebras/ghostpdl.git;a=shortlog), should I make you the default assignee in bugzilla and who_owns_what.txt? | 14:15.57 |
| chrisl | HenryStiles: I warned him that would happen - he's no one to blame but himself! | 14:18.58 |
| HenryStiles | :-) | 14:20.32 |
| sebras | does who_owns_what.txt explain who owns who_owns_what.txt? | 14:34.59 |
| | chrisl: in all honesty, you dod. and I was foolish enough to just want to fix the issue in the annex h bitstream that has been there since jbig2dec's inception. :) | 14:35.41 |
| HenryStiles | shelly also helps out with it but I think it good you own it. | 14:37.17 |
| | sebras: ^^^ | 14:37.33 |
| sebras | HenryStiles: yeah, I can see how it helps if there are urgent issues. | 14:38.28 |
| | HenryStiles: I hope there are none! :) | 14:38.42 |
| Robin_Watts | kens: ping! | 14:42.37 |
| kens | Yep ? | 14:42.43 |
| Robin_Watts | I'm looking at this ps2write thing. | 14:42.56 |
| kens | OK | 14:42.59 |
| Robin_Watts | And I think the problem is that ps2write is capturing image data by making a device just big enough for the image, and getting the image plotting stuff to fill it in. | 14:43.33 |
| kens | Very probably true | 14:43.44 |
| Robin_Watts | So I'm being asked to write to an image device that's 0x28 wide. | 14:43.50 |
| kens | Though I doubt its 'capturing' image data that way | 14:43.54 |
| | I suspect its creating an image device and rendering something like a type 4 image to it | 14:44.09 |
| Robin_Watts | It is a type4 image. | 14:44.16 |
| kens | Because level 2 doesn't support that kind of image | 14:44.18 |
| | Right so the only way we can handle it is to render it | 14:44.37 |
| Robin_Watts | So I get called to plot an image starting at (say) (100,0) | 14:44.56 |
| | The old code plots that by doing a series of fill_rectangles starting at (100,0) | 14:45.27 |
| HenryStiles | Robin_Watts: I left a message about the PXL issue, let me know if I can do more with it. I stepped through your code, but didn't see anything obvious, I can look again. | 14:45.41 |
| Robin_Watts | lcvd_fill_rectangle_shifted gets called, and that shifts the data leftwards to 0,0, and plots it into the device. | 14:45.53 |
| | HenryStiles: Yeah, I saw, thanks. I'll talk to you about that in a bit. | 14:46.09 |
| kens | yeah lcvd is a locacl covnert device or soemthing | 14:46.10 |
| Robin_Watts | kens will head off before HenryStiles :) | 14:46.18 |
| kens | I'm here ofr a bit yet :) | 14:46.29 |
| Robin_Watts | kens: Right, so in the new code, I clip x and y against 0 and dev->width./ | 14:46.44 |
| | and that of course doesn't work. | 14:47.05 |
| kens | Talk to tor for a second while I find the code | 14:47.25 |
| | Robin_Watts : yell when you are ready again. | 14:48.48 |
| Robin_Watts | kens: thanks, sorry. | 14:49.04 |
| kens | NP | 14:49.08 |
| | So I'm not sure why clipping is a problem | 14:49.15 |
| | Oops thought tor was done.... | 14:49.35 |
| kens | waits some more | 14:49.41 |
| HenryStiles | chrisl: do you get the aws emails? The business about switching to longer id's? | 14:50.04 |
| chrisl | HenryStiles: I saw it, I haven't read it properly yet | 14:50.27 |
| | "Systems that parse or store resource IDs...." - I'm not sure we do anything like that | 14:51.27 |
| HenryStiles | chrisl: I believe it is to do with accessing instances from scripts | 14:52.26 |
| | but not sure | 14:52.30 |
| chrisl | Yeh, which I don't think we have anything that does that | 14:52.47 |
| HenryStiles | it will matter for the new containers and arguably we should switch to scripts for security (according to Corin) | 14:53.12 |
| chrisl | Switch what to scripts? | 14:53.38 |
| HenryStiles | accessing and doing everything from the command line instead of using the dashboard panel | 14:54.00 |
| chrisl | We pretty much only use the dashboard to reboot casper.... | 14:54.48 |
| | It seems we have to reboot/relaunch casper to get long IDs, too <sigh> | 14:55.39 |
| HenryStiles | well if we ever need to do a restore we'd use the dash | 14:55.53 |
| chrisl | We couldn't really script that, though. | 14:56.28 |
| | I guess the question is: how much time do we want to spend learning the AWS/ECS APIs, and writing scripts, for the rare occasions we need to access those functions? | 14:57.53 |
| HenryStiles | everything you can do on the dash has a command line equivalent (AWS command line interface) | 14:58.06 |
| chrisl | So, what was the last backup image, then? And would it be the right one to use for a restore? | 14:59.25 |
| Robin_Watts | kens: Ok, I think I'm back. | 15:03.46 |
| kens | :-) | 15:03.50 |
| | So disclaimer; this isn't my code (I blame my repdecessor) and most of what I've done has been around avoiding ever getting here..... | 15:04.18 |
| Robin_Watts | kens: I suspect that your code is fine. | 15:04.27 |
| kens | Given that the lcvd device 'seems' to haev a set of methods for handling images, how come you are bypassing them ? | 15:04.49 |
| Robin_Watts | and that it's my problem for assuming that every device can accept X values between 0 and dev->width | 15:04.58 |
| | kens: actually, maybe it's not my fault. | 15:05.16 |
| kens | Is this down to the fact that you haven#'t defined a device method for this yet ? | 15:05.16 |
| Robin_Watts | no. | 15:05.20 |
| kens | Oh..... | 15:05.27 |
| | I'm missing a vital piece somewhere | 15:05.40 |
| | Sinfe the device seems to 'shift' everything, why is there a problem ? | 15:05.54 |
| Robin_Watts | When you write into a device, I think the convention is that you can reasonably expect the device to accept only pixels written between (0,0) and (dev->width,dev->height) | 15:06.02 |
| | Anything outside that can be assumed to be clipped away. | 15:06.22 |
| | Does that seem fair? | 15:06.28 |
| kens | Broadly, though I've known devices where the raster could be smaller (needs to be a set size, but you can't print to all of it) | 15:07.00 |
| Robin_Watts | kens: OK, but even in that case, anything outside of that range still wouldn't appear. | 15:07.32 |
| kens | Indeed | 15:07.38 |
| Robin_Watts | (0,0) -> (dev->width,dev->height) is an "upper" bound on what will be recognised, if you like. | 15:08.01 |
| kens | Yep | 15:08.05 |
| Robin_Watts | ps2write is presenting a device that goes from (0,0) -> (small, small) | 15:08.21 |
| | say (0,0) -> (100,100) | 15:08.28 |
| kens | Yes | 15:08.30 |
| Robin_Watts | but then pixels sent to (200,200) might get shifted down by (200,200) to be at (0,0) | 15:08.53 |
| kens | Exactly so yes | 15:09.02 |
| Robin_Watts | so the actual pixels accepted by that device are (200,200)->(300,300) | 15:09.05 |
| | which breaks the convention above. | 15:09.14 |
| kens | But it does the shift5ing itself (I think) | 15:09.16 |
| Robin_Watts | It does, but that doesn't help callers who have looked at the device and said "Oh, these pixels are out of range, I won't bother sending them" | 15:09.42 |
| kens | I htink that decision should not be made os early then | 15:10.23 |
| | Its a question of where the actual 'device' ends | 15:10.36 |
| | This exhibits the nature of the dichotomy in Ghostscript | 15:10.50 |
| Robin_Watts | kens: We've just agreed that devices should pass a dev->width and a dev->height that reflects the upper bound of what is acceptable. | 15:11.01 |
| kens | Where chained 'devices' are not actually the terminal device | 15:11.02 |
| Robin_Watts | and now you've changed your mind. | 15:11.20 |
| kens | Not p[recisely | 15:11.28 |
| | Its depends what you categorise as a 'device' | 15:11.36 |
| | Which is what I was trying to express | 15:11.44 |
| | In Ghostscript everything is a 'device' | 15:11.51 |
| Robin_Watts | The "dev" pointer that is given to callers. That's the device in question. | 15:11.56 |
| kens | even itf its intermediate between teh interpreter and the actuual print engine | 15:12.04 |
| | Yeah but that's my point | 15:12.10 |
| | While the actual print engine can reasonably be limited in this way, the intermediate devices need not be | 15:12.32 |
| | In AN Other rip we hade 'devices' and 'device classes' | 15:12.47 |
| | devices were real physical things | 15:12.56 |
| Robin_Watts | The whole point of the device encapsulation is that everyone should be able to assume that a device behaves in the same way. | 15:13.07 |
| kens | device classes were things between the interpreter and actual device | 15:13.11 |
| Robin_Watts | Whether it's an intermediate device or not shouldn't matter. | 15:13.19 |
| kens | I think it does | 15:13.26 |
| Robin_Watts | Clearly, it does not :) | 15:13.33 |
| kens | What hte lcvd device is doing here is not unreasoable | 15:13.35 |
| Robin_Watts | Yes it is. | 15:13.40 |
| kens | For its intended purpose | 15:13.42 |
| | No it isn't | 15:13.46 |
| | Its creating a 'peephole' of the final canvas | 15:13.58 |
| | and rendering to that, but shiftin the peephole so that its moved to 0,0 for tha actual output | 15:14.22 |
| Robin_Watts | Suppose the final canvas is (1000,1000) | 15:14.28 |
| kens | Otherwise it would need to create a potentially large canvas just ot render a small area | 15:14.43 |
| Robin_Watts | and the lvcd device wants to offer a peephole to the middle (500,500)->(600,600) of that. | 15:14.57 |
| | Then it can create a memory image device for (0,0)->(100,100) | 15:15.20 |
| | and it can redirect writes to the lvcd device to the memory image by shifting them down by (500,500). | 15:15.48 |
| kens | I thought that was what it was doing | 15:15.59 |
| Robin_Watts | But the actual lvcd device should still claim to have width/height of 1000/1000, not 100/100. | 15:16.10 |
| kens | Hmmmmm | 15:16.17 |
| Robin_Watts | That last bit is what it's not doing. | 15:16.19 |
| kens | But then clipping might be incorrect ? | 15:16.41 |
| | I htink this same approach is used for shadings | 15:16.53 |
| | And those can extend up to the edge of the raster | 15:17.02 |
| | I suppose creating a clip path at the same time would solve that | 15:17.32 |
| | If that's possible at this level | 15:17.38 |
| Robin_Watts | kens: The caller is absolutely allowed to assume that any writes to a device outside of the (0,0)->(dev->width,dev->height) range will have no effect. | 15:17.38 |
| | and the lvcd device breaks that at the moment. | 15:18.00 |
| kens | Currenlty it works | 15:18.15 |
| Robin_Watts | It bloody doesn't : | 15:18.24 |
| | It bloody doesn't :) | 15:18.26 |
| kens | It doesn't for your code, the existing code works | 15:18.33 |
| | Now I'm not defgending it, but I can see problems wiht altering it | 15:18.45 |
| Robin_Watts | kens: Cos it's getting lucky. | 15:18.51 |
| kens | Potential problems anyway | 15:18.52 |
| | Its been lucky ofr > 10 years | 15:19.01 |
| Robin_Watts | Regardless. | 15:19.08 |
| kens | Brekaing it isn't going to fly | 15:19.17 |
| Robin_Watts | kens: brb, sorry. | 15:19.34 |
| kens | OK | 15:19.38 |
| Robin_Watts | kens: So, what do you forsee as being the problem with having the lvcd device give the proper (full) width/height ? | 15:21.16 |
| kens | I'm not against changing it, but I'm trying to think through the implications. Given that I'm not intimately familiar with the code, I'm concerned that altering the width/height might change the clipping, causing attempted writes within the width/height, but outside the memory device raster. Because a shading can have Extend [true true] which will extend to the edge of the raster | 15:21.23 |
| | ie if we define a memory raster 100x100 but say the width is 1000 then the sadhing plotter might try to write a raster > 100 pixels | 15:22.02 |
| Robin_Watts | kens: The lvcd device would have a different width/height. The underlying memory device would have the true one for the shifted window. | 15:22.11 |
| | At the moment, if I ignored clipping and tried to write a 1000 pixel wide raster to the lvcd device, that'd get shifted and passed through to the memory one. | 15:22.49 |
| kens | Oh, so its the lcvd device declaring a width that's incorrect ? | 15:22.57 |
| Robin_Watts | Yes. | 15:23.02 |
| kens | Right I missed that | 15:23.06 |
| Robin_Watts | The lcvd is saying (100,100) wide, when it should be dev->width,dev->height. | 15:23.20 |
| kens | Again thopough does this mean that by declaring a width of 1000, we'd get attempts to plot outside the actual underlying raster ? | 15:23.32 |
| Robin_Watts | kens: No. | 15:23.50 |
| kens | OK we could throw those away in the lcvd device, but its wasterful to even attempt to plot it | 15:23.53 |
| | Well if you're sure, then OK | 15:24.01 |
| Robin_Watts | I don't see why there should be any problem that we don't already have | 15:24.22 |
| kens | is unsure | 15:24.33 |
| | Not really familiar with the code here | 15:24.40 |
| Robin_Watts | At the moment, if I plot a 1000 pixel wide raster, the lcvd device shifts the origin, but passes on the request to the memory device unchanged. | 15:24.47 |
| | and the memory device clips it. | 15:24.56 |
| kens | Hmm, OK | 15:25.17 |
| | As I say, does thi mean that we will attempt to plot areas which are not in the memory device, and then throw them away ? It just a performance waste. | 15:26.08 |
| | But it seems not unreasonable anyway | 15:26.15 |
| | Now I understand what you're saying better | 15:26.22 |
| Robin_Watts | kens: No more so than we are doing already. | 15:26.43 |
| kens | Then I guess its not a problem | 15:26.51 |
| Robin_Watts | In order to avoid that we'd have to have each device have a left/top as well as the current right/bottom. | 15:27.33 |
| kens | Yeah | 15:27.43 |
| Robin_Watts | kens: So where can we make this change? :) | 15:28.32 |
| kens | I have no idea, I assumed you would know | 15:28.45 |
| | Presumably somewhere in gdevpdfd.c | 15:28.53 |
| Robin_Watts | I can see where lcvd is called. I can't see where it's created. | 15:29.00 |
| kens | gdevpdfd.c | 15:29.08 |
| | line 1323 or so | 15:29.26 |
| | pdf_setup_masked_image_converter | 15:29.34 |
| chrisl | What are mapped_x/y for? | 15:29.58 |
| kens | For shifting from the requested x and y to 0,0 | 15:30.13 |
| | Or so I understand it (possibly incorrectly) | 15:30.28 |
| chrisl | Odd that there's a mapped_height but not width | 15:31.03 |
| kens | There's a mapped height ? | 15:31.25 |
| | Robin_Watts : does that look like the right place ? | 15:32.13 |
| chrisl | In the memory device structure | 15:32.17 |
| kens | Oh, I hadn't looked at the structure | 15:32.27 |
| Robin_Watts | kens: That's where I'm at, yes. | 15:32.30 |
| kens | The code doesn't fill it in | 15:32.32 |
| Robin_Watts | ok, so the problem is that this stuff doesn't make a cvd device. | 15:35.30 |
| | It makes a mem device, and bends it. | 15:35.41 |
| kens | Looks like it yes | 15:35.47 |
| | You might say it wraps soemthing around the device | 15:36.23 |
| | A pretty poor compromise really | 15:36.51 |
| | At a guess I'd say this is because of the limitations in chaining devices | 15:37.46 |
| Robin_Watts | I suspect this has been written to be "just enough" to make existing code work. | 15:38.20 |
| kens | Possibly | 15:38.26 |
| Robin_Watts | And I have some sympathy for that approach. | 15:38.28 |
| kens | But the problem might have been that image enumerators hide stuff away | 15:38.42 |
| | Should have been possible to work around it though | 15:39.02 |
| Robin_Watts | image enumerators don't come into this. usually it's fill_rectangle that gets hit (and now copy_color). | 15:39.27 |
| | Now, I am going to have to add a dso to allow me to disable the whole "go to a buffer -> copy_color" way of working, for the cases where copy_color ->fill_rectangle anyway. | 15:40.22 |
| kens | Umm, what ? | 15:40.41 |
| Robin_Watts | kens: which bit? | 15:40.56 |
| kens | Oh copy color goes to fill rectangle | 15:40.57 |
| Robin_Watts | Yeah, gx_default_copy_color is implemented on top of fill_rectangle | 15:41.13 |
| | So I'll have to add a dso to let me spot that. | 15:41.23 |
| kens | You could just ask 'is it high level' and disable it if ti is | 15:41.33 |
| Robin_Watts | and I can use that to disable it in this case too. | 15:41.38 |
| kens | THat works too | 15:41.43 |
| Robin_Watts | kens: I'd be asking the lcvd device, which isn't high level. :) | 15:41.59 |
| kens | I'm not sure what it replies though | 15:42.13 |
| Robin_Watts | but I'm sure something is possible. | 15:42.17 |
| | The main thing is that I now understand why this is happening. | 15:42.29 |
| kens | The lcvd device does reply to certain spec ops | 15:42.39 |
| | But not high level | 15:42.48 |
| | Might as well use whatever you're going to and implement it there | 15:42.58 |
| Robin_Watts | kens: yeah. | 15:43.08 |
| | Ok, thanks for that. | 15:43.26 |
| kens | NP | 15:43.31 |
| | FOrtunately it was a convenient time :-) | 15:43.40 |
| Robin_Watts | Now I can bug HenryStiles :) | 15:48.24 |
| kens | aya :-) | 15:48.48 |
| HenryStiles | Robin_Watts: I'm surprised you aren't seeing that problem in many more places. It's rare in practice but not in the QL tests | 15:53.25 |
| | Robin_Watts: that said I didn't get to the root of what was going on. | 15:53.49 |
| Robin_Watts | HenryStiles: You suggested that I could just punt on the pattern case, but that's just avoiding something in our code that we don't understand. | 15:54.33 |
| | I strongly suspect that there is a flaw in some of our existing code that we're running into. | 15:54.48 |
| | I found one bug already that has improved the output, but it's still not perfect. | 15:55.12 |
| | (that being a flaw in mem_gray8_rgb24_strip_copy_rop) | 15:55.47 |
| HenryStiles | I wasn't reaching that code with the simple case, but yes I suspect you are right | 15:58.55 |
| Robin_Watts | I will keep bashing. | 16:02.24 |
| | Hmm. what is "get_clipping_box" about? | 16:40.56 |
| | Does that mean my previous statements about "callers of a device can assume that anything outside (0,0) to (dev->width,dev->height) will be ignored" is false ? | 16:41.38 |
| | ray_laptop, HenryStiles, chrisl: ^ | 16:50.24 |
| | There is bog all documentation for get_clipping_box anywhere. | 16:55.13 |
| | I'd assume it was for further constraining the active bbox from the default full device size, rather than expanding it outside that device size. | 16:56.12 |
| HenryStiles | Robin_Watts:are we talking about the code in gxipixel.c, where if there is a clip path we use it otherwise use the device box? | 17:15.07 |
| Robin_Watts | HenryStiles: No, just more generally. | 17:15.24 |
| | It seems to me that it's implicit in the definition of a ghostscript device that any writes to anywhere outside of (0,0) to (dev->width, dev->height) should be guaranteed not to affect the output. | 17:16.07 |
| | Is that reasonable? | 17:16.10 |
| HenryStiles | yes | 17:16.32 |
| Robin_Watts | Ok, so the get_clipping_box device function can only ever reduce that area. | 17:17.18 |
| HenryStiles | Robin_Watts: I would expect the intersection, but as I look at the code I'm confused :-) | 17:26.02 |
| Robin_Watts | HenryStiles: I have a commit that fixes the lcvd device to behave in what I consider a sane manner. | 17:26.29 |
| | It's not the prettiest thing in the world, but it fixes the ps2write issues with my changes (at least the ones I've tested) | 17:26.52 |
| | http://git.ghostscript.com/?p=user/robin/ghostpdl.git;a=commitdiff;h=4c24647c59ef41253cc92ebccaca8b1dba36d8d9 | 17:27.17 |
| HenryStiles | Robin_Watts: I must be missing something because the way I read the code (before your changes) the get_clipping_box() code should return (500,500, 600, 600) using your example, that is what you mean by "tell the world" right? The result of calling that function. There are some typos lvcd ... | 18:09.57 |
| Robin_Watts | get_clipping_box returns the right thing. | 18:10.28 |
| | But dev->width and dev->height are both 100. | 18:10.41 |
| | so my code, which assumes that anything outside of (0,0) and (100,100) won't mark, doesn't bother sending the image data. | 18:11.09 |
| | I was concerned that perhaps my expectations were wrong, and that rather than looking at dev->width, dev->height I should have been looking purely at get_clipping_box. | 18:12.06 |
| | but I now think that looking at dev->width/dev->height is fine. | 18:12.31 |
| | I could possibly do *better* by looking at get_clipping_box and intersecting it, but there is no need for that to be correct. | 18:13.00 |
| HenryStiles | width and height aren't coordinates though they are distances so it's not clear to me the device is wrong. But I guess we have those assumptions all over | 18:13.08 |
| Robin_Watts | woah. | 18:13.18 |
| | width/height aren't coordinates? | 18:13.29 |
| | I think, as you say, everywhere in the code we assume that devices run from (0,0) to (dev->width, dev->height) | 18:14.07 |
| | So they are coordinates, effectively. | 18:14.25 |
| HenryStiles | I agree, yes | 18:14.34 |
| Robin_Watts | If we have a (dev->origin.x, dev->origin.y), then they wouldn't be (and this would be a whole different conversation). | 18:14.51 |
| | but we don't :) | 18:14.59 |
| | I'll fix the typos, thanks. | 18:15.12 |
| | Crap, I get segv's with my patch :( | 18:15.46 |
| | Forward 1 day (to 2018/04/26)>>> | |
Log of #ghostscript at irc.freenode.net.